Re: [PATCH 1/6 net-next] inet: collapse ipv4/v6 rcv_saddr_equal functions into one

2 messages, 2 authors, 2017-01-12 · open the first message on its own page

Re: [PATCH 1/6 net-next] inet: collapse ipv4/v6 rcv_saddr_equal functions into one

From: Craig Gallek <hidden>
Date: 2017-01-12 17:41:37

On Wed, Jan 11, 2017 at 3:19 PM, Josef Bacik [off-list ref] wrote:
+int inet_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2,
+                        bool match_wildcard)
+{
+#if IS_ENABLED(CONFIG_IPV6)
+       if (sk->sk_family == AF_INET6)
Still wrapping my head around this, so take it with a grain of salt,
but it's not obvious to me that sk and sk2 are guaranteed to have the
same family here (or if it even matters).  Especially in the context
of the next patch which removes the bind_conflict callback...  Does
this need to be an OR check for the family of either socket?  Or is it
safe as-is because the first socket passed to this function is always
the existing one and the second one is the possible conflict socket?

Re: [PATCH 1/6 net-next] inet: collapse ipv4/v6 rcv_saddr_equal functions into one

From: Josef Bacik <hidden>
Date: 2017-01-12 18:14:31

On Thu, Jan 12, 2017 at 12:41 PM, Craig Gallek [off-list ref] 
wrote:
On Wed, Jan 11, 2017 at 3:19 PM, Josef Bacik [off-list ref] wrote:
quoted
 +int inet_rcv_saddr_equal(const struct sock *sk, const struct sock 
*sk2,
 +                        bool match_wildcard)
 +{
 +#if IS_ENABLED(CONFIG_IPV6)
 +       if (sk->sk_family == AF_INET6)
Still wrapping my head around this, so take it with a grain of salt,
but it's not obvious to me that sk and sk2 are guaranteed to have the
same family here (or if it even matters).  Especially in the context
of the next patch which removes the bind_conflict callback...  Does
this need to be an OR check for the family of either socket?  Or is it
safe as-is because the first socket passed to this function is always
the existing one and the second one is the possible conflict socket?
It's safe as is, all we care is that sk1 is the INET6 socket.  In the 
compare function we use inet6_rcv_saddr(sk2) which will return NULL if 
it isn't INET6 and then the function handles that case appropriately.  
This stuff is subtle so it's easy to get confused, I always made sure 
to run it on our production boxes to make sure I didn't break something 
;).  Thanks,

Josef
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help