| Message ID | 2c9abbd7ac3b89af9addb550bccb9169f47e39a2.1575779993.git.lucien.xin@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: nft_tunnel: reinforce key opts support | expand |
On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote: > info->key.tp_src and tp_dst are __be16, when using nla_put_be16() > to dump them, htons() is not needed, so remove it in this patch. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/netfilter/nft_tunnel.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c > index d9d6c0d..e1184fa 100644 > --- a/net/netfilter/nft_tunnel.c > +++ b/net/netfilter/nft_tunnel.c > @@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb, > static int nft_tunnel_ports_dump(struct sk_buff *skb, > struct ip_tunnel_info *info) > { > - if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 || > - nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0) > + if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 || > + nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0) info->key.tp_src is already in __be16 as you describe. So I would take this as a consistency fix. I would take this as a fix.
On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote: > info->key.tp_src and tp_dst are __be16, when using nla_put_be16() > to dump them, htons() is not needed, so remove it in this patch. > > Signed-off-by: Xin Long <lucien.xin@gmail.com> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
On Wed, Dec 11, 2019 at 10:53:12PM +0100, Pablo Neira Ayuso wrote: > On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote: > > info->key.tp_src and tp_dst are __be16, when using nla_put_be16() > > to dump them, htons() is not needed, so remove it in this patch. > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > --- > > net/netfilter/nft_tunnel.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c > > index d9d6c0d..e1184fa 100644 > > --- a/net/netfilter/nft_tunnel.c > > +++ b/net/netfilter/nft_tunnel.c > > @@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb, > > static int nft_tunnel_ports_dump(struct sk_buff *skb, > > struct ip_tunnel_info *info) > > { > > - if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 || > > - nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0) > > + if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 || > > + nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0) > > info->key.tp_src is already in __be16 as you describe. So I would take > this as a consistency fix. I would take this as a fix. I mean, I think this is worth fixing it indeed.
On Wed, Dec 11, 2019 at 11:06:37PM +0100, Pablo Neira Ayuso wrote: > On Wed, Dec 11, 2019 at 10:53:12PM +0100, Pablo Neira Ayuso wrote: > > On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote: > > > info->key.tp_src and tp_dst are __be16, when using nla_put_be16() > > > to dump them, htons() is not needed, so remove it in this patch. > > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > > --- > > > net/netfilter/nft_tunnel.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c > > > index d9d6c0d..e1184fa 100644 > > > --- a/net/netfilter/nft_tunnel.c > > > +++ b/net/netfilter/nft_tunnel.c > > > @@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb, > > > static int nft_tunnel_ports_dump(struct sk_buff *skb, > > > struct ip_tunnel_info *info) > > > { > > > - if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 || > > > - nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0) > > > + if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 || > > > + nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0) > > > > info->key.tp_src is already in __be16 as you describe. So I would take > > this as a consistency fix. I would take this as a fix. > > I mean, I think this is worth fixing it indeed. Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>
diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c index d9d6c0d..e1184fa 100644 --- a/net/netfilter/nft_tunnel.c +++ b/net/netfilter/nft_tunnel.c @@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb, static int nft_tunnel_ports_dump(struct sk_buff *skb, struct ip_tunnel_info *info) { - if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 || - nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0) + if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 || + nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0) return -1; return 0;
info->key.tp_src and tp_dst are __be16, when using nla_put_be16() to dump them, htons() is not needed, so remove it in this patch. Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/netfilter/nft_tunnel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)