| Message ID | 20200308011930.6923-1-ap420073@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | None | expand |
On Sun, Mar 08, 2020 at 01:19:30AM +0000, Taehee Yoo wrote: > In the current code, udp_encap_enable() is called in > bareudp_socket_create(). > But, setup_udp_tunnel_sock() internally calls udp_encap_enable(). > So, udp_encap_enable() is unnecessary. > > Signed-off-by: Taehee Yoo <ap420073@gmail.com> > --- > drivers/net/bareudp.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c > index c9d0d68467f7..71a2f480f70e 100644 > --- a/drivers/net/bareudp.c > +++ b/drivers/net/bareudp.c > @@ -250,9 +250,6 @@ static int bareudp_socket_create(struct bareudp_dev *bareudp, __be16 port) > tunnel_cfg.encap_destroy = NULL; > setup_udp_tunnel_sock(bareudp->net, sock, &tunnel_cfg); > > - if (sock->sk->sk_family == AF_INET6) > - udp_encap_enable(); > - udp_encap_enable is not called for V6 sockets so we need to have the above lines of code > rcu_assign_pointer(bareudp->sock, sock); > return 0; > } > -- > 2.17.1 >
On Mon, 9 Mar 2020 at 15:03, Martin Varghese <martinvarghesenokia@gmail.com> wrote: > Hi Martin, Thanek you for the review! > On Sun, Mar 08, 2020 at 01:19:30AM +0000, Taehee Yoo wrote: > > In the current code, udp_encap_enable() is called in > > bareudp_socket_create(). > > But, setup_udp_tunnel_sock() internally calls udp_encap_enable(). > > So, udp_encap_enable() is unnecessary. > > > > Signed-off-by: Taehee Yoo <ap420073@gmail.com> > > --- > > drivers/net/bareudp.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c > > index c9d0d68467f7..71a2f480f70e 100644 > > --- a/drivers/net/bareudp.c > > +++ b/drivers/net/bareudp.c > > @@ -250,9 +250,6 @@ static int bareudp_socket_create(struct bareudp_dev *bareudp, __be16 port) > > tunnel_cfg.encap_destroy = NULL; > > setup_udp_tunnel_sock(bareudp->net, sock, &tunnel_cfg); > > > > - if (sock->sk->sk_family == AF_INET6) > > - udp_encap_enable(); > > - > udp_encap_enable is not called for V6 sockets so we need to have the above lines of code This patch is already merged into net-next. So, you could send a revert patch. In addition, I'm not familiar with the socket API. So I'm a little bit curious about why you didn't create separated ipv4 and ipv6 sockets? Vxlan, geneve, etc create separated ipv4, ipv6 UDP socket. In the bareudp modules, it creates a single socket and it tries to call both udp_encap_enable() and udpv6_encap_enable. Is there any special reason or both two ways are actually the same things? Thank you so much! Taehee Yoo
diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c index c9d0d68467f7..71a2f480f70e 100644 --- a/drivers/net/bareudp.c +++ b/drivers/net/bareudp.c @@ -250,9 +250,6 @@ static int bareudp_socket_create(struct bareudp_dev *bareudp, __be16 port) tunnel_cfg.encap_destroy = NULL; setup_udp_tunnel_sock(bareudp->net, sock, &tunnel_cfg); - if (sock->sk->sk_family == AF_INET6) - udp_encap_enable(); - rcu_assign_pointer(bareudp->sock, sock); return 0; }
In the current code, udp_encap_enable() is called in bareudp_socket_create(). But, setup_udp_tunnel_sock() internally calls udp_encap_enable(). So, udp_encap_enable() is unnecessary. Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- drivers/net/bareudp.c | 3 --- 1 file changed, 3 deletions(-)