* [PATCH net] ipv4: fix wildcard search with inet_confirm_addr()
@ 2013-11-08 14:37 Nicolas Dichtel
2013-11-08 21:24 ` Julian Anastasov
0 siblings, 1 reply; 7+ messages in thread
From: Nicolas Dichtel @ 2013-11-08 14:37 UTC (permalink / raw)
To: davem; +Cc: netdev, Nicolas Dichtel
Help of this function says: "in_dev: only on this interface, 0=any interface",
but in fact the code supposes that it will never be NULL.
Note that this function was never called with in_dev == NULL, but it's exported
and may be used by an external module.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
This bug was spotted by code review.
drivers/net/bonding/bonding.h | 10 +++-------
include/linux/inetdevice.h | 3 ++-
net/ipv4/arp.c | 3 ++-
net/ipv4/devinet.c | 8 ++++----
4 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 03cf3fd14490..448895e90ccb 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -407,15 +407,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be32 local)
{
- struct in_device *in_dev;
- __be32 addr = 0;
+ __be32 addr;
rcu_read_lock();
- in_dev = __in_dev_get_rcu(dev);
-
- if (in_dev)
- addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
-
+ addr = inet_confirm_addr(dev_net(dev), __in_dev_get_rcu(dev), dst,
+ local, RT_SCOPE_HOST);
rcu_read_unlock();
return addr;
}
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 79640e015a86..5870f7060917 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
extern void devinet_init(void);
extern struct in_device *inetdev_by_index(struct net *, int);
extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
-extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope);
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
+ __be32 local, int scope);
extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask);
static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7808093cede6..fc5ebc7e1962 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -408,7 +408,8 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
default:
return 0;
}
- return !inet_confirm_addr(in_dev, sip, tip, scope);
+ return !inet_confirm_addr(dev_net(in_dev->dev), in_dev, sip, tip,
+ scope);
}
static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index a1b5bcbd04ae..47cdb035f817 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1236,22 +1236,22 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
/*
* Confirm that local IP address exists using wildcards:
+ * - net: netns to check, cannot be NULL
* - in_dev: only on this interface, 0=any interface
* - dst: only in the same subnet as dst, 0=any dst
* - local: address, 0=autoselect the local address
* - scope: maximum allowed scope value for the local address
*/
-__be32 inet_confirm_addr(struct in_device *in_dev,
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
__be32 dst, __be32 local, int scope)
{
__be32 addr = 0;
struct net_device *dev;
- struct net *net;
if (scope != RT_SCOPE_LINK)
- return confirm_addr_indev(in_dev, dst, local, scope);
+ return in_dev ? confirm_addr_indev(in_dev, dst, local, scope) :
+ 0;
- net = dev_net(in_dev->dev);
rcu_read_lock();
for_each_netdev_rcu(net, dev) {
in_dev = __in_dev_get_rcu(dev);
--
1.8.4.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net] ipv4: fix wildcard search with inet_confirm_addr()
2013-11-08 14:37 [PATCH net] ipv4: fix wildcard search with inet_confirm_addr() Nicolas Dichtel
@ 2013-11-08 21:24 ` Julian Anastasov
2013-11-13 15:23 ` [PATCH net v2] " Nicolas Dichtel
0 siblings, 1 reply; 7+ messages in thread
From: Julian Anastasov @ 2013-11-08 21:24 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev
Hello,
On Fri, 8 Nov 2013, Nicolas Dichtel wrote:
> Help of this function says: "in_dev: only on this interface, 0=any interface",
> but in fact the code supposes that it will never be NULL.
>
> Note that this function was never called with in_dev == NULL, but it's exported
> and may be used by an external module.
The initial idea was that arp_ignore=3 can provide
dev=NULL. Later argument was changed to in_dev but we should
allow the NULL value. As you are going to change the arguments
may be we can partially revert commit
39a6d06300128d32f361f4f790beba0ca83730eb, i.e. to add the
missing in_dev = NULL in arp_ignore() and also revert this change:
- if (in_dev != NULL)
+ if (scope != RT_SCOPE_LINK)
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>
> This bug was spotted by code review.
>
> drivers/net/bonding/bonding.h | 10 +++-------
> include/linux/inetdevice.h | 3 ++-
> net/ipv4/arp.c | 3 ++-
> net/ipv4/devinet.c | 8 ++++----
> 4 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 03cf3fd14490..448895e90ccb 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -407,15 +407,11 @@ static inline bool bond_is_slave_inactive(struct slave *slave)
>
> static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be32 local)
> {
> - struct in_device *in_dev;
> - __be32 addr = 0;
> + __be32 addr;
>
> rcu_read_lock();
> - in_dev = __in_dev_get_rcu(dev);
> -
> - if (in_dev)
> - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
> -
> + addr = inet_confirm_addr(dev_net(dev), __in_dev_get_rcu(dev), dst,
> + local, RT_SCOPE_HOST);
As in_dev = NULL should be allowed, above change
will not be needed, we should keep the 'if (in_dev)' there.
> rcu_read_unlock();
> return addr;
> }
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index 79640e015a86..5870f7060917 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
> extern void devinet_init(void);
> extern struct in_device *inetdev_by_index(struct net *, int);
> extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
> -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope);
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
> + __be32 local, int scope);
> extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask);
>
> static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7808093cede6..fc5ebc7e1962 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -408,7 +408,8 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> default:
> return 0;
> }
> - return !inet_confirm_addr(in_dev, sip, tip, scope);
> + return !inet_confirm_addr(dev_net(in_dev->dev), in_dev, sip, tip,
> + scope);
We will need initial struct net *net = dev_net(in_dev->dev)
because later we will set in_dev to NULL for arp_ignore=3.
Then we will provide 'net' to inet_confirm_addr.
> }
>
> static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index a1b5bcbd04ae..47cdb035f817 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1236,22 +1236,22 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
>
> /*
> * Confirm that local IP address exists using wildcards:
> + * - net: netns to check, cannot be NULL
> * - in_dev: only on this interface, 0=any interface
NULL is better than 0
> * - dst: only in the same subnet as dst, 0=any dst
> * - local: address, 0=autoselect the local address
> * - scope: maximum allowed scope value for the local address
> */
> -__be32 inet_confirm_addr(struct in_device *in_dev,
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
> __be32 dst, __be32 local, int scope)
> {
> __be32 addr = 0;
> struct net_device *dev;
> - struct net *net;
>
> if (scope != RT_SCOPE_LINK)
Use if (in_dev)
> - return confirm_addr_indev(in_dev, dst, local, scope);
> + return in_dev ? confirm_addr_indev(in_dev, dst, local, scope) :
> + 0;
We will not need the above change.
> - net = dev_net(in_dev->dev);
> rcu_read_lock();
> for_each_netdev_rcu(net, dev) {
> in_dev = __in_dev_get_rcu(dev);
> --
> 1.8.4.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net v2] ipv4: fix wildcard search with inet_confirm_addr()
2013-11-08 21:24 ` Julian Anastasov
@ 2013-11-13 15:23 ` Nicolas Dichtel
2013-11-13 21:27 ` Julian Anastasov
0 siblings, 1 reply; 7+ messages in thread
From: Nicolas Dichtel @ 2013-11-13 15:23 UTC (permalink / raw)
To: ja; +Cc: davem, netdev, Nicolas Dichtel
Help of this function says: "in_dev: only on this interface, 0=any interface",
but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
correct namespace."), the code supposes that it will never be NULL. This
function is never called with in_dev == NULL, but it's exported and may be used
by an external module.
Because this patch restore the ability to call inet_confirm_addr() with in_dev
== NULL, I partially revert the above commit, as suggested by Julian.
CC: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
This bug was spotted by code review.
v2: fix change in bond_confirm_addr()
partially revert 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
correct namespace.")
fix API desc of inet_confirm_addr()
drivers/net/bonding/bonding.h | 4 ++--
include/linux/inetdevice.h | 3 ++-
net/ipv4/arp.c | 4 +++-
net/ipv4/devinet.c | 9 ++++-----
4 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 03cf3fd14490..0ad3f4bd99c0 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -414,8 +414,8 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3
in_dev = __in_dev_get_rcu(dev);
if (in_dev)
- addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
-
+ addr = inet_confirm_addr(dev_net(dev), in_dev, dst, local,
+ RT_SCOPE_HOST);
rcu_read_unlock();
return addr;
}
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 79640e015a86..5870f7060917 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
extern void devinet_init(void);
extern struct in_device *inetdev_by_index(struct net *, int);
extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
-extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope);
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
+ __be32 local, int scope);
extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask);
static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7808093cede6..46c7b4483bcf 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -379,6 +379,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
{
+ struct net *net = dev_net(in_dev->dev);
int scope;
switch (IN_DEV_ARP_IGNORE(in_dev)) {
@@ -397,6 +398,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
case 3: /* Do not reply for scope host addresses */
sip = 0;
scope = RT_SCOPE_LINK;
+ in_dev = NULL;
break;
case 4: /* Reserved */
case 5:
@@ -408,7 +410,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
default:
return 0;
}
- return !inet_confirm_addr(in_dev, sip, tip, scope);
+ return !inet_confirm_addr(net, in_dev, sip, tip, scope);
}
static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index a1b5bcbd04ae..19426e4b232c 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1236,22 +1236,21 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
/*
* Confirm that local IP address exists using wildcards:
- * - in_dev: only on this interface, 0=any interface
+ * - net: netns to check, cannot be NULL
+ * - in_dev: only on this interface, NULL=any interface
* - dst: only in the same subnet as dst, 0=any dst
* - local: address, 0=autoselect the local address
* - scope: maximum allowed scope value for the local address
*/
-__be32 inet_confirm_addr(struct in_device *in_dev,
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
__be32 dst, __be32 local, int scope)
{
__be32 addr = 0;
struct net_device *dev;
- struct net *net;
- if (scope != RT_SCOPE_LINK)
+ if (in_dev != NULL)
return confirm_addr_indev(in_dev, dst, local, scope);
- net = dev_net(in_dev->dev);
rcu_read_lock();
for_each_netdev_rcu(net, dev) {
in_dev = __in_dev_get_rcu(dev);
--
1.8.4.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net v2] ipv4: fix wildcard search with inet_confirm_addr()
2013-11-13 15:23 ` [PATCH net v2] " Nicolas Dichtel
@ 2013-11-13 21:27 ` Julian Anastasov
2013-11-14 10:32 ` Nicolas Dichtel
2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel
0 siblings, 2 replies; 7+ messages in thread
From: Julian Anastasov @ 2013-11-13 21:27 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: davem, netdev
Hello,
On Wed, 13 Nov 2013, Nicolas Dichtel wrote:
> Help of this function says: "in_dev: only on this interface, 0=any interface",
> but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
> correct namespace."), the code supposes that it will never be NULL. This
> function is never called with in_dev == NULL, but it's exported and may be used
> by an external module.
>
> Because this patch restore the ability to call inet_confirm_addr() with in_dev
> == NULL, I partially revert the above commit, as suggested by Julian.
>
> CC: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
This patch looks ok to me, thanks!
Reviewed-by: Julian Anastasov <ja@ssi.bg>
Still, I think this is a net-next material
and you have to wait some days... The net tree that
you are using is missing the changes that remove
'extern', so I think you have to port it to net-next
tree to avoid the conflict in include/linux/inetdevice.h.
> ---
>
> This bug was spotted by code review.
>
> v2: fix change in bond_confirm_addr()
> partially revert 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
> correct namespace.")
> fix API desc of inet_confirm_addr()
>
> drivers/net/bonding/bonding.h | 4 ++--
> include/linux/inetdevice.h | 3 ++-
> net/ipv4/arp.c | 4 +++-
> net/ipv4/devinet.c | 9 ++++-----
> 4 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 03cf3fd14490..0ad3f4bd99c0 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -414,8 +414,8 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3
> in_dev = __in_dev_get_rcu(dev);
>
> if (in_dev)
> - addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
> -
> + addr = inet_confirm_addr(dev_net(dev), in_dev, dst, local,
> + RT_SCOPE_HOST);
> rcu_read_unlock();
> return addr;
> }
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index 79640e015a86..5870f7060917 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -164,7 +164,8 @@ extern int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
> extern void devinet_init(void);
> extern struct in_device *inetdev_by_index(struct net *, int);
> extern __be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
> -extern __be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local, int scope);
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
> + __be32 local, int scope);
> extern struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix, __be32 mask);
>
> static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7808093cede6..46c7b4483bcf 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -379,6 +379,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
>
> static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> {
> + struct net *net = dev_net(in_dev->dev);
> int scope;
>
> switch (IN_DEV_ARP_IGNORE(in_dev)) {
> @@ -397,6 +398,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> case 3: /* Do not reply for scope host addresses */
> sip = 0;
> scope = RT_SCOPE_LINK;
> + in_dev = NULL;
> break;
> case 4: /* Reserved */
> case 5:
> @@ -408,7 +410,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
> default:
> return 0;
> }
> - return !inet_confirm_addr(in_dev, sip, tip, scope);
> + return !inet_confirm_addr(net, in_dev, sip, tip, scope);
> }
>
> static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index a1b5bcbd04ae..19426e4b232c 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -1236,22 +1236,21 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
>
> /*
> * Confirm that local IP address exists using wildcards:
> - * - in_dev: only on this interface, 0=any interface
> + * - net: netns to check, cannot be NULL
> + * - in_dev: only on this interface, NULL=any interface
> * - dst: only in the same subnet as dst, 0=any dst
> * - local: address, 0=autoselect the local address
> * - scope: maximum allowed scope value for the local address
> */
> -__be32 inet_confirm_addr(struct in_device *in_dev,
> +__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
> __be32 dst, __be32 local, int scope)
> {
> __be32 addr = 0;
> struct net_device *dev;
> - struct net *net;
>
> - if (scope != RT_SCOPE_LINK)
> + if (in_dev != NULL)
> return confirm_addr_indev(in_dev, dst, local, scope);
>
> - net = dev_net(in_dev->dev);
> rcu_read_lock();
> for_each_netdev_rcu(net, dev) {
> in_dev = __in_dev_get_rcu(dev);
> --
> 1.8.4.1
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net v2] ipv4: fix wildcard search with inet_confirm_addr()
2013-11-13 21:27 ` Julian Anastasov
@ 2013-11-14 10:32 ` Nicolas Dichtel
2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel
1 sibling, 0 replies; 7+ messages in thread
From: Nicolas Dichtel @ 2013-11-14 10:32 UTC (permalink / raw)
To: Julian Anastasov; +Cc: davem, netdev
Le 13/11/2013 22:27, Julian Anastasov a écrit :
>
> Hello,
>
> On Wed, 13 Nov 2013, Nicolas Dichtel wrote:
>
>> Help of this function says: "in_dev: only on this interface, 0=any interface",
>> but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
>> correct namespace."), the code supposes that it will never be NULL. This
>> function is never called with in_dev == NULL, but it's exported and may be used
>> by an external module.
>>
>> Because this patch restore the ability to call inet_confirm_addr() with in_dev
>> == NULL, I partially revert the above commit, as suggested by Julian.
>>
>> CC: Julian Anastasov <ja@ssi.bg>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> This patch looks ok to me, thanks!
>
> Reviewed-by: Julian Anastasov <ja@ssi.bg>
>
> Still, I think this is a net-next material
> and you have to wait some days... The net tree that
Ok.
> you are using is missing the changes that remove
> 'extern', so I think you have to port it to net-next
> tree to avoid the conflict in include/linux/inetdevice.h.
Yes, checkpatch already warns about this extern kerword.
Thank you,
Nicolas
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next v3] ipv4: fix wildcard search with inet_confirm_addr()
2013-11-13 21:27 ` Julian Anastasov
2013-11-14 10:32 ` Nicolas Dichtel
@ 2013-12-10 14:02 ` Nicolas Dichtel
2013-12-11 19:49 ` David Miller
1 sibling, 1 reply; 7+ messages in thread
From: Nicolas Dichtel @ 2013-12-10 14:02 UTC (permalink / raw)
To: ja; +Cc: davem, netdev, Nicolas Dichtel
Help of this function says: "in_dev: only on this interface, 0=any interface",
but since commit 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
correct namespace."), the code supposes that it will never be NULL. This
function is never called with in_dev == NULL, but it's exported and may be used
by an external module.
Because this patch restore the ability to call inet_confirm_addr() with in_dev
== NULL, I partially revert the above commit, as suggested by Julian.
CC: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Reviewed-by: Julian Anastasov <ja@ssi.bg>
---
This was spotted by code review.
v3: rebase patch on net-next
v2: fix change in bond_confirm_addr()
partially revert 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
correct namespace.")
fix API desc of inet_confirm_addr()
drivers/net/bonding/bonding.h | 4 ++--
include/linux/inetdevice.h | 5 ++---
net/ipv4/arp.c | 4 +++-
net/ipv4/devinet.c | 9 ++++-----
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index a9f4f9f4d8ce..a74c92c83ead 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -394,8 +394,8 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3
in_dev = __in_dev_get_rcu(dev);
if (in_dev)
- addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);
-
+ addr = inet_confirm_addr(dev_net(dev), in_dev, dst, local,
+ RT_SCOPE_HOST);
rcu_read_unlock();
return addr;
}
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index ae174ca565c9..72edb60e0072 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -164,11 +164,10 @@ int devinet_ioctl(struct net *net, unsigned int cmd, void __user *);
void devinet_init(void);
struct in_device *inetdev_by_index(struct net *, int);
__be32 inet_select_addr(const struct net_device *dev, __be32 dst, int scope);
-__be32 inet_confirm_addr(struct in_device *in_dev, __be32 dst, __be32 local,
- int scope);
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
+ __be32 local, int scope);
struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix,
__be32 mask);
-
static __inline__ int inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
{
return !((addr^ifa->ifa_address)&ifa->ifa_mask);
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index f04185863940..b67cf805910e 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -381,6 +381,7 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
{
+ struct net *net = dev_net(in_dev->dev);
int scope;
switch (IN_DEV_ARP_IGNORE(in_dev)) {
@@ -399,6 +400,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
case 3: /* Do not reply for scope host addresses */
sip = 0;
scope = RT_SCOPE_LINK;
+ in_dev = NULL;
break;
case 4: /* Reserved */
case 5:
@@ -410,7 +412,7 @@ static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
default:
return 0;
}
- return !inet_confirm_addr(in_dev, sip, tip, scope);
+ return !inet_confirm_addr(net, in_dev, sip, tip, scope);
}
static int arp_filter(__be32 sip, __be32 tip, struct net_device *dev)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 43065be36301..7316ad1dfda5 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1238,22 +1238,21 @@ static __be32 confirm_addr_indev(struct in_device *in_dev, __be32 dst,
/*
* Confirm that local IP address exists using wildcards:
- * - in_dev: only on this interface, 0=any interface
+ * - net: netns to check, cannot be NULL
+ * - in_dev: only on this interface, NULL=any interface
* - dst: only in the same subnet as dst, 0=any dst
* - local: address, 0=autoselect the local address
* - scope: maximum allowed scope value for the local address
*/
-__be32 inet_confirm_addr(struct in_device *in_dev,
+__be32 inet_confirm_addr(struct net *net, struct in_device *in_dev,
__be32 dst, __be32 local, int scope)
{
__be32 addr = 0;
struct net_device *dev;
- struct net *net;
- if (scope != RT_SCOPE_LINK)
+ if (in_dev != NULL)
return confirm_addr_indev(in_dev, dst, local, scope);
- net = dev_net(in_dev->dev);
rcu_read_lock();
for_each_netdev_rcu(net, dev) {
in_dev = __in_dev_get_rcu(dev);
--
1.8.4.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v3] ipv4: fix wildcard search with inet_confirm_addr()
2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel
@ 2013-12-11 19:49 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2013-12-11 19:49 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: ja, netdev
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 10 Dec 2013 15:02:40 +0100
> 39a6d0630012 ("[NETNS]: Process inet_confirm_addr in the
> correct namespace."), the code supposes that it will never be NULL. This
> function is never called with in_dev == NULL, but it's exported and may be used
> by an external module.
>
> Because this patch restore the ability to call inet_confirm_addr() with in_dev
> == NULL, I partially revert the above commit, as suggested by Julian.
>
> CC: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Reviewed-by: Julian Anastasov <ja@ssi.bg>
I personally think that we may be over analyzing this, because if some
external module actually cared, someone would have complained in the
last _5_ years as that's how long the new semantics have been in
place.
Nevertheless, passing in the network namespace explicitly is a bit
nicer than having special behavior for RT_SCOPE_LINE, thus I've
applied this.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-12-11 19:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-08 14:37 [PATCH net] ipv4: fix wildcard search with inet_confirm_addr() Nicolas Dichtel
2013-11-08 21:24 ` Julian Anastasov
2013-11-13 15:23 ` [PATCH net v2] " Nicolas Dichtel
2013-11-13 21:27 ` Julian Anastasov
2013-11-14 10:32 ` Nicolas Dichtel
2013-12-10 14:02 ` [PATCH net-next v3] " Nicolas Dichtel
2013-12-11 19:49 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.