[Patch net-next 0/5] fou: some fixes and updates

[Patch net-next 0/5] fou: some fixes and updates

[Cong Wang]

Patch 1~3 fix some bugs in net/ipv4/fou.c, the only
thing I am not sure is if it's too late to change the
byte order of FOU_ATTR_PORT, if so we have to fix iproute2
instead of kernel.

Patch 4~5 add some new features to make it more complete.


Cong Wang (5):
  fou: avoid calling udp_del_offload() twice
  fou: exit early when parsing config fails
  fou: fix byte order of udp_config.local_udp_port
  fou: add network namespace support
  fou: implement FOU_CMD_GET

 include/uapi/linux/fou.h |   1 +
 net/ipv4/fou.c           | 230 +++++++++++++++++++++++++++++++++++++----------
 2 files changed, 186 insertions(+), 45 deletions(-)

-- 
1.8.3.1

[Patch net-next 1/5] fou: avoid calling udp_del_offload() twice

[Cong Wang]

This fixes the following harmless warning:

./ip/ip fou del port 7777
[  122.907516] udp_del_offload: didn't find offload for port 7777

Cc: Tom Herbert <therbert@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/ipv4/fou.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index ff069f6..c8db627 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -410,7 +410,8 @@ static void fou_release(struct fou *fou)
 	struct socket *sock = fou->sock;
 	struct sock *sk = sock->sk;
 
-	udp_del_offload(&fou->udp_offloads);
+	if (sk->sk_family == AF_INET)
+		udp_del_offload(&fou->udp_offloads);
 
 	list_del(&fou->list);
 
@@ -528,7 +529,6 @@ static int fou_destroy(struct net *net, struct fou_cfg *cfg)
 	spin_lock(&fou_lock);
 	list_for_each_entry(fou, &fou_list, list) {
 		if (fou->port == port) {
-			udp_del_offload(&fou->udp_offloads);
 			fou_release(fou);
 			err = 0;
 			break;
-- 
1.8.3.1

[Patch net-next 2/5] fou: exit early when parsing config fails

[Cong Wang]

Cc: Tom Herbert <therbert@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/ipv4/fou.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index c8db627..ad0ee82 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -605,8 +605,11 @@ static int fou_nl_cmd_add_port(struct sk_buff *skb, struct genl_info *info)
 static int fou_nl_cmd_rm_port(struct sk_buff *skb, struct genl_info *info)
 {
 	struct fou_cfg cfg;
+	int err;
 
-	parse_nl_config(info, &cfg);
+	err = parse_nl_config(info, &cfg);
+	if (err)
+		return err;
 
 	return fou_destroy(&init_net, &cfg);
 }
-- 
1.8.3.1

[Patch net-next 3/5] fou: fix byte order of udp_config.local_udp_port

[Cong Wang]

udp_config.local_udp_port is be16. And iproute2 passes
network order for FOU_ATTR_PORT.

Cc: Tom Herbert <therbert@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/ipv4/fou.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index ad0ee82..cf606fc 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -468,7 +468,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 	sk = sock->sk;
 
 	fou->flags = cfg->flags;
-	fou->port = cfg->udp_config.local_udp_port;
+	fou->port = ntohs(cfg->udp_config.local_udp_port);
 
 	/* Initial for fou type */
 	switch (cfg->type) {
@@ -523,7 +523,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 static int fou_destroy(struct net *net, struct fou_cfg *cfg)
 {
 	struct fou *fou;
-	u16 port = cfg->udp_config.local_udp_port;
+	u16 port = ntohs(cfg->udp_config.local_udp_port);
 	int err = -EINVAL;
 
 	spin_lock(&fou_lock);
@@ -573,7 +573,7 @@ static int parse_nl_config(struct genl_info *info,
 	}
 
 	if (info->attrs[FOU_ATTR_PORT]) {
-		u16 port = nla_get_u16(info->attrs[FOU_ATTR_PORT]);
+		__be16 port = nla_get_be16(info->attrs[FOU_ATTR_PORT]);
 
 		cfg->udp_config.local_udp_port = port;
 	}
-- 
1.8.3.1

[Patch net-next 4/5] fou: add network namespace support

[Cong Wang]

Also convert the spinlock to a mutex.

Cc: Tom Herbert <therbert@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/ipv4/fou.c | 106 ++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 67 insertions(+), 39 deletions(-)

diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index cf606fc..61ce417 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -16,9 +16,6 @@
 #include <uapi/linux/fou.h>
 #include <uapi/linux/genetlink.h>
 
-static DEFINE_SPINLOCK(fou_lock);
-static LIST_HEAD(fou_list);
-
 struct fou {
 	struct socket *sock;
 	u8 protocol;
@@ -37,6 +34,13 @@ struct fou_cfg {
 	struct udp_port_cfg udp_config;
 };
 
+static unsigned int fou_net_id;
+
+struct fou_net {
+	struct list_head fou_list;
+	struct mutex fou_lock;
+};
+
 static inline struct fou *fou_from_sock(struct sock *sk)
 {
 	return sk->sk_user_data;
@@ -387,20 +391,21 @@ static int gue_gro_complete(struct sk_buff *skb, int nhoff,
 	return err;
 }
 
-static int fou_add_to_port_list(struct fou *fou)
+static int fou_add_to_port_list(struct net *net, struct fou *fou)
 {
+	struct fou_net *fn = net_generic(net, fou_net_id);
 	struct fou *fout;
 
-	spin_lock(&fou_lock);
-	list_for_each_entry(fout, &fou_list, list) {
+	mutex_lock(&fn->fou_lock);
+	list_for_each_entry(fout, &fn->fou_list, list) {
 		if (fou->port == fout->port) {
-			spin_unlock(&fou_lock);
+			mutex_unlock(&fn->fou_lock);
 			return -EALREADY;
 		}
 	}
 
-	list_add(&fou->list, &fou_list);
-	spin_unlock(&fou_lock);
+	list_add(&fou->list, &fn->fou_list);
+	mutex_unlock(&fn->fou_lock);
 
 	return 0;
 }
@@ -412,13 +417,8 @@ static void fou_release(struct fou *fou)
 
 	if (sk->sk_family == AF_INET)
 		udp_del_offload(&fou->udp_offloads);
-
 	list_del(&fou->list);
-
-	/* Remove hooks into tunnel socket */
-	sk->sk_user_data = NULL;
-
-	sock_release(sock);
+	udp_tunnel_sock_release(sock);
 
 	kfree(fou);
 }
@@ -448,10 +448,10 @@ static int gue_encap_init(struct sock *sk, struct fou *fou, struct fou_cfg *cfg)
 static int fou_create(struct net *net, struct fou_cfg *cfg,
 		      struct socket **sockp)
 {
-	struct fou *fou = NULL;
-	int err;
 	struct socket *sock = NULL;
+	struct fou *fou = NULL;
 	struct sock *sk;
+	int err;
 
 	/* Open UDP socket */
 	err = udp_sock_create(net, &cfg->udp_config, &sock);
@@ -503,7 +503,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 			goto error;
 	}
 
-	err = fou_add_to_port_list(fou);
+	err = fou_add_to_port_list(net, fou);
 	if (err)
 		goto error;
 
@@ -515,26 +515,27 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 error:
 	kfree(fou);
 	if (sock)
-		sock_release(sock);
+		udp_tunnel_sock_release(sock);
 
 	return err;
 }
 
 static int fou_destroy(struct net *net, struct fou_cfg *cfg)
 {
-	struct fou *fou;
+	struct fou_net *fn = net_generic(net, fou_net_id);
 	u16 port = ntohs(cfg->udp_config.local_udp_port);
 	int err = -EINVAL;
+	struct fou *fou;
 
-	spin_lock(&fou_lock);
-	list_for_each_entry(fou, &fou_list, list) {
+	mutex_lock(&fn->fou_lock);
+	list_for_each_entry(fou, &fn->fou_list, list) {
 		if (fou->port == port) {
 			fou_release(fou);
 			err = 0;
 			break;
 		}
 	}
-	spin_unlock(&fou_lock);
+	mutex_unlock(&fn->fou_lock);
 
 	return err;
 }
@@ -592,6 +593,7 @@ static int parse_nl_config(struct genl_info *info,
 
 static int fou_nl_cmd_add_port(struct sk_buff *skb, struct genl_info *info)
 {
+	struct net *net = genl_info_net(info);
 	struct fou_cfg cfg;
 	int err;
 
@@ -599,11 +601,12 @@ static int fou_nl_cmd_add_port(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		return err;
 
-	return fou_create(&init_net, &cfg, NULL);
+	return fou_create(net, &cfg, NULL);
 }
 
 static int fou_nl_cmd_rm_port(struct sk_buff *skb, struct genl_info *info)
 {
+	struct net *net = genl_info_net(info);
 	struct fou_cfg cfg;
 	int err;
 
@@ -611,7 +614,7 @@ static int fou_nl_cmd_rm_port(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		return err;
 
-	return fou_destroy(&init_net, &cfg);
+	return fou_destroy(net, &cfg);
 }
 
 static const struct genl_ops fou_nl_ops[] = {
@@ -823,38 +826,63 @@ static void ip_tunnel_encap_del_fou_ops(void)
 
 #endif
 
+static __net_init int fou_init_net(struct net *net)
+{
+	struct fou_net *fn = net_generic(net, fou_net_id);
+
+	INIT_LIST_HEAD(&fn->fou_list);
+	mutex_init(&fn->fou_lock);
+	return 0;
+}
+
+static __net_exit void fou_exit_net(struct net *net)
+{
+	struct fou_net *fn = net_generic(net, fou_net_id);
+	struct fou *fou, *next;
+
+	/* Close all the FOU sockets */
+	mutex_lock(&fn->fou_lock);
+	list_for_each_entry_safe(fou, next, &fn->fou_list, list)
+		fou_release(fou);
+	mutex_unlock(&fn->fou_lock);
+}
+
+static struct pernet_operations fou_net_ops = {
+	.init = fou_init_net,
+	.exit = fou_exit_net,
+	.id   = &fou_net_id,
+	.size = sizeof(struct fou_net),
+};
+
 static int __init fou_init(void)
 {
 	int ret;
 
+	ret = register_pernet_device(&fou_net_ops);
+	if (ret)
+		goto exit;
+
 	ret = genl_register_family_with_ops(&fou_nl_family,
 					    fou_nl_ops);
-
 	if (ret < 0)
-		goto exit;
+		goto unregister;
 
 	ret = ip_tunnel_encap_add_fou_ops();
-	if (ret < 0)
-		genl_unregister_family(&fou_nl_family);
+	if (ret == 0)
+		return 0;
 
+	genl_unregister_family(&fou_nl_family);
+unregister:
+	unregister_pernet_device(&fou_net_ops);
 exit:
 	return ret;
 }
 
 static void __exit fou_fini(void)
 {
-	struct fou *fou, *next;
-
 	ip_tunnel_encap_del_fou_ops();
-
 	genl_unregister_family(&fou_nl_family);
-
-	/* Close all the FOU sockets */
-
-	spin_lock(&fou_lock);
-	list_for_each_entry_safe(fou, next, &fou_list, list)
-		fou_release(fou);
-	spin_unlock(&fou_lock);
+	unregister_pernet_device(&fou_net_ops);
 }
 
 module_init(fou_init);
-- 
1.8.3.1

[Patch net-next 5/5] fou: implement FOU_CMD_GET

[Cong Wang]

Cc: Tom Herbert <therbert@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/uapi/linux/fou.h |   1 +
 net/ipv4/fou.c           | 109 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/include/uapi/linux/fou.h b/include/uapi/linux/fou.h
index c303588..d2947c5 100644
--- a/include/uapi/linux/fou.h
+++ b/include/uapi/linux/fou.h
@@ -25,6 +25,7 @@ enum {
 	FOU_CMD_UNSPEC,
 	FOU_CMD_ADD,
 	FOU_CMD_DEL,
+	FOU_CMD_GET,
 
 	__FOU_CMD_MAX,
 };
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 61ce417..23dfb3b 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -21,6 +21,7 @@ struct fou {
 	u8 protocol;
 	u8 flags;
 	u16 port;
+	u16 type;
 	struct udp_offload udp_offloads;
 	struct list_head list;
 };
@@ -487,6 +488,8 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
 		goto error;
 	}
 
+	fou->type = cfg->type;
+
 	udp_sk(sk)->encap_type = 1;
 	udp_encap_enable();
 
@@ -617,6 +620,106 @@ static int fou_nl_cmd_rm_port(struct sk_buff *skb, struct genl_info *info)
 	return fou_destroy(net, &cfg);
 }
 
+static int fou_fill_info(struct fou *fou, struct sk_buff *msg)
+{
+	if (nla_put_u8(msg, FOU_ATTR_AF, fou->sock->sk->sk_family) ||
+	    nla_put_be16(msg, FOU_ATTR_PORT, htons(fou->port)) ||
+	    nla_put_u8(msg, FOU_ATTR_IPPROTO, fou->protocol) ||
+	    nla_put_u8(msg, FOU_ATTR_TYPE, fou->type))
+		return -1;
+
+	if (fou->flags & FOU_F_REMCSUM_NOPARTIAL)
+		if (nla_put_flag(msg, FOU_ATTR_REMCSUM_NOPARTIAL))
+			return -1;
+	return 0;
+}
+
+static int fou_dump_info(struct fou *fou, u32 portid, u32 seq,
+			 u32 flags, struct sk_buff *skb, u8 cmd)
+{
+	void *hdr;
+
+	hdr = genlmsg_put(skb, portid, seq, &fou_nl_family, flags, cmd);
+	if (!hdr)
+		return -ENOMEM;
+
+	if (fou_fill_info(fou, skb) < 0)
+		goto nla_put_failure;
+
+	genlmsg_end(skb, hdr);
+	return 0;
+
+nla_put_failure:
+	genlmsg_cancel(skb, hdr);
+	return -EMSGSIZE;
+}
+
+static int fou_nl_cmd_get_port(struct sk_buff *skb, struct genl_info *info)
+{
+	struct net *net = genl_info_net(info);
+	struct fou_net *fn = net_generic(net, fou_net_id);
+	struct sk_buff *msg;
+	struct fou_cfg cfg;
+	struct fou *fout;
+	u16 port;
+	int ret;
+
+	ret = parse_nl_config(info, &cfg);
+	if (ret)
+		return ret;
+	port = ntohs(cfg.udp_config.local_udp_port);
+	if (port == 0)
+		return -EINVAL;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	ret = -ESRCH;
+	mutex_lock(&fn->fou_lock);
+	list_for_each_entry(fout, &fn->fou_list, list) {
+		if (port == fout->port) {
+			ret = fou_dump_info(fout, info->snd_portid,
+					    info->snd_seq, 0, msg,
+					    info->genlhdr->cmd);
+			break;
+		}
+	}
+	mutex_unlock(&fn->fou_lock);
+	if (ret < 0)
+		goto out_free;
+
+	return genlmsg_reply(msg, info);
+
+out_free:
+	nlmsg_free(msg);
+	return ret;
+}
+
+static int fou_nl_dump(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	struct net *net = sock_net(skb->sk);
+	struct fou_net *fn = net_generic(net, fou_net_id);
+	struct fou *fout;
+	int idx = 0, ret;
+
+	mutex_lock(&fn->fou_lock);
+	list_for_each_entry(fout, &fn->fou_list, list) {
+		if (idx++ < cb->args[0])
+			continue;
+		ret = fou_dump_info(fout, NETLINK_CB(cb->skb).portid,
+				    cb->nlh->nlmsg_seq, NLM_F_MULTI,
+				    skb, FOU_CMD_GET);
+		if (ret)
+			goto done;
+	}
+	mutex_unlock(&fn->fou_lock);
+
+done:
+	cb->args[0] = idx;
+	return skb->len;
+}
+
 static const struct genl_ops fou_nl_ops[] = {
 	{
 		.cmd = FOU_CMD_ADD,
@@ -630,6 +733,12 @@ static const struct genl_ops fou_nl_ops[] = {
 		.policy = fou_nl_policy,
 		.flags = GENL_ADMIN_PERM,
 	},
+	{
+		.cmd = FOU_CMD_GET,
+		.doit = fou_nl_cmd_get_port,
+		.dumpit = fou_nl_dump,
+		.policy = fou_nl_policy,
+	},
 };
 
 size_t fou_encap_hlen(struct ip_tunnel_encap *e)
-- 
1.8.3.1

Re: [Patch net-next 3/5] fou: fix byte order of udp_config.local_udp_port

[Tom Herbert]

On Mon, Apr 6, 2015 at 4:41 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> udp_config.local_udp_port is be16. And iproute2 passes
> network order for FOU_ATTR_PORT.
>
> Cc: Tom Herbert <therbert@google.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  net/ipv4/fou.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
> index ad0ee82..cf606fc 100644
> --- a/net/ipv4/fou.c
> +++ b/net/ipv4/fou.c
> @@ -468,7 +468,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
>         sk = sock->sk;
>
>         fou->flags = cfg->flags;
> -       fou->port = cfg->udp_config.local_udp_port;
> +       fou->port = ntohs(cfg->udp_config.local_udp_port);
>
Cong, I don't think this is right. Port should always be nbo, probably
should be using __b16 instead of u16 for defines. Did you test these
changes?

>         /* Initial for fou type */
>         switch (cfg->type) {
> @@ -523,7 +523,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
>  static int fou_destroy(struct net *net, struct fou_cfg *cfg)
>  {
>         struct fou *fou;
> -       u16 port = cfg->udp_config.local_udp_port;
> +       u16 port = ntohs(cfg->udp_config.local_udp_port);
>         int err = -EINVAL;
>
>         spin_lock(&fou_lock);
> @@ -573,7 +573,7 @@ static int parse_nl_config(struct genl_info *info,
>         }
>
>         if (info->attrs[FOU_ATTR_PORT]) {
> -               u16 port = nla_get_u16(info->attrs[FOU_ATTR_PORT]);
> +               __be16 port = nla_get_be16(info->attrs[FOU_ATTR_PORT]);
>
>                 cfg->udp_config.local_udp_port = port;
>         }
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch net-next 3/5] fou: fix byte order of udp_config.local_udp_port

[Cong Wang]

On Mon, Apr 6, 2015 at 11:09 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Mon, Apr 6, 2015 at 4:41 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
>> index ad0ee82..cf606fc 100644
>> --- a/net/ipv4/fou.c
>> +++ b/net/ipv4/fou.c
>> @@ -468,7 +468,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
>>         sk = sock->sk;
>>
>>         fou->flags = cfg->flags;
>> -       fou->port = cfg->udp_config.local_udp_port;
>> +       fou->port = ntohs(cfg->udp_config.local_udp_port);
>>
> Cong, I don't think this is right. Port should always be nbo, probably
> should be using __b16 instead of u16 for defines. Did you test these
> changes?
>

Yeah, fou->port is u16 which should be host order, however
we only save and compare it with be16, so the original code
is not mis-behaving (otherwise should go to -net), just reads odd.

I tested it together with the whole patchset. And I agree we can
change fou->port to be16 too.

I will wait for more feedback before sending v2.

Thanks!

Re: [Patch net-next 3/5] fou: fix byte order of udp_config.local_udp_port

[Tom Herbert]

On Tue, Apr 7, 2015 at 2:11 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Mon, Apr 6, 2015 at 11:09 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Mon, Apr 6, 2015 at 4:41 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
>>> index ad0ee82..cf606fc 100644
>>> --- a/net/ipv4/fou.c
>>> +++ b/net/ipv4/fou.c
>>> @@ -468,7 +468,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
>>>         sk = sock->sk;
>>>
>>>         fou->flags = cfg->flags;
>>> -       fou->port = cfg->udp_config.local_udp_port;
>>> +       fou->port = ntohs(cfg->udp_config.local_udp_port);
>>>
>> Cong, I don't think this is right. Port should always be nbo, probably
>> should be using __b16 instead of u16 for defines. Did you test these
>> changes?
>>
>
> Yeah, fou->port is u16 which should be host order, however
> we only save and compare it with be16, so the original code
> is not mis-behaving (otherwise should go to -net), just reads odd.
>
> I tested it together with the whole patchset. And I agree we can
> change fou->port to be16 too.

Hmm,I don't understand how this could have worked with the ntohs() added.

> I will wait for more feedback before sending v2.
>
> Thanks!

Re: [Patch net-next 3/5] fou: fix byte order of udp_config.local_udp_port

[Cong Wang]

On Tue, Apr 7, 2015 at 2:47 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Tue, Apr 7, 2015 at 2:11 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Mon, Apr 6, 2015 at 11:09 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> On Mon, Apr 6, 2015 at 4:41 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>> diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
>>>> index ad0ee82..cf606fc 100644
>>>> --- a/net/ipv4/fou.c
>>>> +++ b/net/ipv4/fou.c
>>>> @@ -468,7 +468,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
>>>>         sk = sock->sk;
>>>>
>>>>         fou->flags = cfg->flags;
>>>> -       fou->port = cfg->udp_config.local_udp_port;
>>>> +       fou->port = ntohs(cfg->udp_config.local_udp_port);
>>>>
>>> Cong, I don't think this is right. Port should always be nbo, probably
>>> should be using __b16 instead of u16 for defines. Did you test these
>>> changes?
>>>
>>
>> Yeah, fou->port is u16 which should be host order, however
>> we only save and compare it with be16, so the original code
>> is not mis-behaving (otherwise should go to -net), just reads odd.
>>
>> I tested it together with the whole patchset. And I agree we can
>> change fou->port to be16 too.
>
> Hmm,I don't understand how this could have worked with the ntohs() added.
>

I must miss something here.

Why converting be16 to u16 with ntohs() doesn't work? After this
patch, all u16 ports have host order, all be16 ports have network order
and we don't mix them.

fou->port is used as a key to lookup all fou's in fou_list and fou_list
is not exposed.

Re: [Patch net-next 3/5] fou: fix byte order of udp_config.local_udp_port

[Tom Herbert]

On Tue, Apr 7, 2015 at 2:57 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Apr 7, 2015 at 2:47 PM, Tom Herbert <tom@herbertland.com> wrote:
>> On Tue, Apr 7, 2015 at 2:11 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Mon, Apr 6, 2015 at 11:09 PM, Tom Herbert <tom@herbertland.com> wrote:
>>>> On Mon, Apr 6, 2015 at 4:41 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>>> diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
>>>>> index ad0ee82..cf606fc 100644
>>>>> --- a/net/ipv4/fou.c
>>>>> +++ b/net/ipv4/fou.c
>>>>> @@ -468,7 +468,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
>>>>>         sk = sock->sk;
>>>>>
>>>>>         fou->flags = cfg->flags;
>>>>> -       fou->port = cfg->udp_config.local_udp_port;
>>>>> +       fou->port = ntohs(cfg->udp_config.local_udp_port);
>>>>>
>>>> Cong, I don't think this is right. Port should always be nbo, probably
>>>> should be using __b16 instead of u16 for defines. Did you test these
>>>> changes?
>>>>
>>>
>>> Yeah, fou->port is u16 which should be host order, however
>>> we only save and compare it with be16, so the original code
>>> is not mis-behaving (otherwise should go to -net), just reads odd.
>>>
>>> I tested it together with the whole patchset. And I agree we can
>>> change fou->port to be16 too.
>>
>> Hmm,I don't understand how this could have worked with the ntohs() added.
>>
>
> I must miss something here.
>
> Why converting be16 to u16 with ntohs() doesn't work? After this
> patch, all u16 ports have host order, all be16 ports have network order
> and we don't mix them.
>
> fou->port is used as a key to lookup all fou's in fou_list and fou_list
> is not exposed.

It should be okay to use nbo for that also.