On Tue, Nov 10, 2015 at 06:30:34PM +0000, Patrick McHardy wrote:
> On 10.11, Pablo Neira Ayuso wrote:
> > With the conversion of the counter expressions to make it percpu, we
> > need to clone the percpu memory area, otherwise we crash when using
> > counters from flow tables.
> >
> > Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
> > ---
> > include/net/netfilter/nf_tables.h | 16 +++++++++++--
> > net/netfilter/nft_counter.c | 49
> > ++++++++++++++++++++++++++++++++-------
> > net/netfilter/nft_dynset.c | 5 ++--
> > 3 files changed, 58 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/net/netfilter/nf_tables.h
> > b/include/net/netfilter/nf_tables.h
> > index c9149cc..c186457 100644
> > --- a/include/net/netfilter/nf_tables.h
> > +++ b/include/net/netfilter/nf_tables.h
> > @@ -630,6 +630,8 @@ struct nft_expr_ops {
> > int (*validate)(const struct nft_ctx *ctx,
> > const struct nft_expr *expr,
> > const struct nft_data
> > **data);
> > + int (*clone)(struct nft_expr *dst,
> > + const struct nft_expr *src);
>
> The functions and data needed during runtime are deliberately kept together
> at the beginning of the structure to avoid having to read the entire thing.
> So I'd say this shoud go after ->eval().
OK, I'll place this after ->eval.
> > @@ -660,10 +662,20 @@ void nft_expr_destroy(const struct nft_ctx *ctx,
> > struct nft_expr *expr);
> > int nft_expr_dump(struct sk_buff *skb, unsigned int attr,
> > const struct nft_expr *expr);
> >
> > -static inline void nft_expr_clone(struct nft_expr *dst, struct nft_expr
> > *src)
> > +static inline int nft_expr_clone(struct nft_expr *dst, struct nft_expr
> > *src)
> > {
> > + int err;
> > +
> > __module_get(src->ops->type->owner);
> > - memcpy(dst, src, src->ops->size);
> > + if (src->ops->clone) {
> > + memcpy(dst, src, sizeof(*src));
>
> Why copy if we clone? The function should do a full initialization if it is
> present I would say.
This is not copying the variable length data area of the expression,
just the expression head.
> > + err = src->ops->clone(dst, src);
> > + if (err < 0)
> > + return err;
> > + } else {
> > + memcpy(dst, src, src->ops->size);
> > + }
> > + return 0;
> > }
> >
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
