On Wed, Mar 06, 2019 at 06:49:12PM +0100, Fernando Fernandez Mancera wrote: > On 3/6/19 5:55 PM, Fernando Fernandez Mancera wrote: > > Sorry about the wrong mailing list, it was a mistake. I am fine with > > your comments so I am going to send a v2. Thanks. > > > > On 3/6/19 5:15 PM, Pablo Neira Ayuso wrote: > >> Ups, wrong mailing list. For patches use > >> netfilter-devel@xxxxxxxxxxxxxxx, so patchwork catches these patches: > >> > >> http://patchwork.ozlabs.org/project/netfilter-devel/list/ > >> > >> Anyway, no worries, comments below. > >> > >> On Wed, Mar 06, 2019 at 01:44:35PM +0100, Fernando Fernandez Mancera wrote: > >>> Add version option support to the nftables "osf" expression. > >>> > >>> Signed-off-by: Fernando Fernandez Mancera <ffmancera@xxxxxxxxxx> > >>> --- > >>> include/linux/netfilter/nfnetlink_osf.h | 11 ++++++--- > >>> include/uapi/linux/netfilter/nf_tables.h | 6 +++++ > >>> net/netfilter/nfnetlink_osf.c | 14 +++++------ > >>> net/netfilter/nft_osf.c | 31 ++++++++++++++++++++---- > >>> 4 files changed, 47 insertions(+), 15 deletions(-) > >>> > >>> diff --git a/include/linux/netfilter/nfnetlink_osf.h b/include/linux/netfilter/nfnetlink_osf.h > >>> index c6000046c966..788613f36935 100644 > >>> --- a/include/linux/netfilter/nfnetlink_osf.h > >>> +++ b/include/linux/netfilter/nfnetlink_osf.h > >>> @@ -21,13 +21,18 @@ struct nf_osf_finger { > >>> struct nf_osf_user_finger finger; > >>> }; > >>> > [...] > >>> > >>> #endif /* _NFOSF_H */ > >>> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h > >>> index a66c8de006cc..39bbb79ef4c2 100644 > >>> --- a/include/uapi/linux/netfilter/nf_tables.h > >>> +++ b/include/uapi/linux/netfilter/nf_tables.h > >>> @@ -1522,15 +1522,21 @@ enum nft_flowtable_hook_attributes { > >>> * > >>> * @NFTA_OSF_DREG: destination register (NLA_U32: nft_registers) > >>> * @NFTA_OSF_TTL: Value of the TTL osf option (NLA_U8) > >>> + * @NFTA_OSF_FLAGS: flags (NLA_U8) > >> > >> Please, use NLA_U32 for this. > >> > > Do you mean all osf "flags" attribute right? So I am going to do the > proper changes in nft, libnftnl and nf-next. Yes. Use NLA_U32 for flags. Remember to deal with endianess via ntohl() and htonl(). > [...] > >>> @@ -47,6 +57,7 @@ static int nft_osf_init(const struct nft_ctx *ctx, > >>> const struct nlattr * const tb[]) > >>> { > >>> struct nft_osf *priv = nft_expr_priv(expr); > >>> + u8 flags; > >>> int err; > >>> u8 ttl; > >>> > >>> @@ -57,6 +68,13 @@ static int nft_osf_init(const struct nft_ctx *ctx, > >>> priv->ttl = ttl; > >>> } > >>> > >>> + if (tb[NFTA_OSF_FLAGS]) { > >>> + flags = nla_get_u8(tb[NFTA_OSF_FLAGS]); > >> > >> If you turn this into a NLA_U32, then you have to use here. > >> > >> flags = ntohl(nla_get_be32(tb[NFTA_OSF_FLAGS])); > >> > >>> + if (flags != NFT_OSF_F_VERSION && flags != 0) > >> > >> No need to restrict flags != 0, remove that part of the check. > >> > > I think we need this restriction because if we do not set "version" > option the "flags" value is 0. If we remove that part of the check we > are forcing to use "version" option always. We can send the NFTA_OSF_FLAGS attribute from userspace, but set it to zero, it's valid to set flags to zero, actually this is the default value (when no NFTA_OSF_FLAGS is sent from userspace).
