| Message ID | 1492471495-13073-1-git-send-email-subashab@codeaurora.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
> -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] > On Behalf Of Subash Abhinov Kasiviswanathan > Sent: Tuesday, April 18, 2017 7:25 AM > To: dsa@cumulusnetworks.com; davem@davemloft.net; > netdev@vger.kernel.org; rshearma@brocade.com; eric.dumazet@gmail.com > Cc: Subash Abhinov Kasiviswanathan; Eric Dumazet > Subject: [PATCH net-next v2] net: ipv6: Fix UDP early demux lookup with > udp_l3mdev_accept=0 > > - return sk; > + udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) { > + if (INET6_MATCH(sk, net, rmt_addr, loc_addr, ports, dif)) > + return sk; > + break; I think break here should remove ? > + } > + return NULL; > } > -- > 1.9.1
>> + break; > I think break here should remove ? Hi Yuan This is similar to __udp4_lib_demux_lookup where we need to check if the first socket is an exact match or break since chains maybe long. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
On 4/17/17 7:07 PM, Subash Abhinov Kasiviswanathan wrote: >>> + break; >> I think break here should remove ? > > Hi Yuan > > This is similar to __udp4_lib_demux_lookup where we need to check if the > first > socket is an exact match or break since chains maybe long. > I suggest adding the same comment as __udp4_lib_demux_lookup; it does look odd.
On 4/17/17 5:24 PM, Subash Abhinov Kasiviswanathan wrote: > David Ahern reported that 5425077d73e0c ("net: ipv6: Add early demux > handler for UDP unicast") breaks udp_l3mdev_accept=0 since early > demux for IPv6 UDP was doing a generic socket lookup which does not > require an exact match. Fix this by making UDPv6 early demux match > connected sockets only. > > v1->v2: Take reference to socket after match as suggested by Eric > > Fixes: 5425077d73e0c ("net: ipv6: Add early demux handler for UDP unicast") > Reported-by: David Ahern <dsa@cumulusnetworks.com> > Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> > Cc: Eric Dumazet <edumazet@google.com> > --- > net/ipv6/udp.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) Besides adding the comment before the break, this looks fine to me. Acked-by: David Ahern <dsa@cumulusnetworks.com> Tested-by: David Ahern <dsa@cumulusnetworks.com> If the only change for v3 is the comment before the break, please keep the above with the new patch.
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index b793ed1..5a4504b 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -46,6 +46,7 @@ #include <net/tcp_states.h> #include <net/ip6_checksum.h> #include <net/xfrm.h> +#include <net/inet_hashtables.h> #include <net/inet6_hashtables.h> #include <net/busy_poll.h> #include <net/sock_reuseport.h> @@ -864,21 +865,25 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable, return 0; } + static struct sock *__udp6_lib_demux_lookup(struct net *net, __be16 loc_port, const struct in6_addr *loc_addr, __be16 rmt_port, const struct in6_addr *rmt_addr, int dif) { + unsigned short hnum = ntohs(loc_port); + unsigned int hash2 = udp6_portaddr_hash(net, loc_addr, hnum); + unsigned int slot2 = hash2 & udp_table.mask; + struct udp_hslot *hslot2 = &udp_table.hash2[slot2]; + const __portpair ports = INET_COMBINED_PORTS(rmt_port, hnum); struct sock *sk; - rcu_read_lock(); - sk = __udp6_lib_lookup(net, rmt_addr, rmt_port, loc_addr, loc_port, - dif, &udp_table, NULL); - if (sk && !atomic_inc_not_zero(&sk->sk_refcnt)) - sk = NULL; - rcu_read_unlock(); - - return sk; + udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) { + if (INET6_MATCH(sk, net, rmt_addr, loc_addr, ports, dif)) + return sk; + break; + } + return NULL; } static void udp_v6_early_demux(struct sk_buff *skb) @@ -903,7 +908,7 @@ static void udp_v6_early_demux(struct sk_buff *skb) else return; - if (!sk) + if (!sk || !atomic_inc_not_zero_hint(&sk->sk_refcnt, 2)) return; skb->sk = sk;
David Ahern reported that 5425077d73e0c ("net: ipv6: Add early demux handler for UDP unicast") breaks udp_l3mdev_accept=0 since early demux for IPv6 UDP was doing a generic socket lookup which does not require an exact match. Fix this by making UDPv6 early demux match connected sockets only. v1->v2: Take reference to socket after match as suggested by Eric Fixes: 5425077d73e0c ("net: ipv6: Add early demux handler for UDP unicast") Reported-by: David Ahern <dsa@cumulusnetworks.com> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> Cc: Eric Dumazet <edumazet@google.com> --- net/ipv6/udp.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)