Re: [PATCH net-next 6/9] net: Fix up inet_addr_type checks
From: David Ahern d...@cumulusnetworks.com
Date: Mon, 10 Aug 2015 11:50:33 -0600
@@ -427,6 +428,7 @@ int inet_bind(struct socket *sock, struct sockaddr
*uaddr, int addr_len)
struct net *net = sock_net(sk);
unsigned short snum;
int chk_addr_ret;
+ int tb_id = 0;
int err;
/* If the socket has its own bind function then use it. (RAW) */
@@ -448,7 +450,16 @@ int inet_bind(struct socket *sock, struct sockaddr
*uaddr, int addr_len)
goto out;
}
- chk_addr_ret = inet_addr_type(net, addr-sin_addr.s_addr);
+ if (sk-sk_bound_dev_if) {
+ struct net_device *dev;
+
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(net, sk-sk_bound_dev_if);
+ if (dev)
+ tb_id = vrf_dev_table_rcu(dev);
+ rcu_read_unlock();
+ }
+ chk_addr_ret = inet_addr_type_table(net, addr-sin_addr.s_addr, tb_id);
/* Not specified by any standard per-se, however it breaks too
* many applications when removed. It is unfortunate since
...
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index b11321a8e58d..d84ae0e30369 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -226,6 +226,9 @@ static inline unsigned int __inet_dev_addr_type(struct
net *net,
rcu_read_lock();
+ if (!tb_id)
+ tb_id = RT_TABLE_LOCAL;
+
table = fib_get_table(net, tb_id);
All of this code that quietly translates table ID zero into RT_TABLE_LOCAL is
confusing.
It would be so much easier to understand if the code was structured like:
int tb_id = RT_TABLE_LOCAL;
if (doing_vrf_stuff)
tb_id = foo;
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 6/9] net: Fix up inet_addr_type checks
On 8/11/15 12:14 PM, David Miller wrote:
From: David Ahern d...@cumulusnetworks.com
Date: Mon, 10 Aug 2015 11:50:33 -0600
@@ -427,6 +428,7 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr,
int addr_len)
struct net *net = sock_net(sk);
unsigned short snum;
int chk_addr_ret;
+ int tb_id = 0;
int err;
/* If the socket has its own bind function then use it. (RAW) */
@@ -448,7 +450,16 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr,
int addr_len)
goto out;
}
- chk_addr_ret = inet_addr_type(net, addr-sin_addr.s_addr);
+ if (sk-sk_bound_dev_if) {
+ struct net_device *dev;
+
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(net, sk-sk_bound_dev_if);
+ if (dev)
+ tb_id = vrf_dev_table_rcu(dev);
+ rcu_read_unlock();
+ }
+ chk_addr_ret = inet_addr_type_table(net, addr-sin_addr.s_addr, tb_id);
/* Not specified by any standard per-se, however it breaks too
* many applications when removed. It is unfortunate since
...
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index b11321a8e58d..d84ae0e30369 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -226,6 +226,9 @@ static inline unsigned int __inet_dev_addr_type(struct net
*net,
rcu_read_lock();
+ if (!tb_id)
+ tb_id = RT_TABLE_LOCAL;
+
table = fib_get_table(net, tb_id);
All of this code that quietly translates table ID zero into RT_TABLE_LOCAL is
confusing.
It would be so much easier to understand if the code was structured like:
int tb_id = RT_TABLE_LOCAL;
if (doing_vrf_stuff)
tb_id = foo;
The intent here was to default to current behavior and to keep the
details of that in one place. If you prefer table id to always enter
with the right value I can make that happen.
David
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next 6/9] net: Fix up inet_addr_type checks
From: David Ahern d...@cumulusnetworks.com Date: Tue, 11 Aug 2015 12:18:20 -0600 The intent here was to default to current behavior and to keep the details of that in one place. If you prefer table id to always enter with the right value I can make that happen. I think it looks better that way. People reading individual pieces of code can tell what is happening much more easily. As currently structured, the have to know the internal details of a helper function to understand what '0' means. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next 6/9] net: Fix up inet_addr_type checks
Currently inet_addr_type and inet_dev_addr_type expect local addresses
to be in the local table. With the VRF device local routes for devices
associated with a VRF will be in the table associated with the VRF.
Provide an alternate inet_addr lookup to use a specific table rather
than defaulting to the local table.
inet_addr_type_dev_table keeps the same semantics as inet_addr_type but
if the passed in device is enslaved to a VRF then the table for that VRF
is used for the lookup.
Signed-off-by: David Ahern d...@cumulusnetworks.com
---
include/net/route.h | 3 +++
net/ipv4/af_inet.c | 13 -
net/ipv4/arp.c | 15 +--
net/ipv4/fib_frontend.c | 28 +---
net/ipv4/fib_semantics.c | 6 --
net/ipv4/icmp.c | 5 +++--
6 files changed, 56 insertions(+), 14 deletions(-)
diff --git a/include/net/route.h b/include/net/route.h
index 6ba681f0b98d..6dda2c1bf8c6 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -192,6 +192,9 @@ unsigned int inet_addr_type(struct net *net, __be32 addr);
unsigned int inet_addr_type_table(struct net *net, __be32 addr, int tb_id);
unsigned int inet_dev_addr_type(struct net *net, const struct net_device *dev,
__be32 addr);
+unsigned int inet_addr_type_dev_table(struct net *net,
+ const struct net_device *dev,
+ __be32 addr);
void ip_rt_multicast_event(struct in_device *);
int ip_rt_ioctl(struct net *, unsigned int cmd, void __user *arg);
void ip_rt_get_source(u8 *src, struct sk_buff *skb, struct rtable *rt);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index cc4e498a0ccf..96fba4f63454 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -119,6 +119,7 @@
#ifdef CONFIG_IP_MROUTE
#include linux/mroute.h
#endif
+#include net/vrf.h
/* The inetsw table contains everything that inet_create needs to
@@ -427,6 +428,7 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr,
int addr_len)
struct net *net = sock_net(sk);
unsigned short snum;
int chk_addr_ret;
+ int tb_id = 0;
int err;
/* If the socket has its own bind function then use it. (RAW) */
@@ -448,7 +450,16 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr,
int addr_len)
goto out;
}
- chk_addr_ret = inet_addr_type(net, addr-sin_addr.s_addr);
+ if (sk-sk_bound_dev_if) {
+ struct net_device *dev;
+
+ rcu_read_lock();
+ dev = dev_get_by_index_rcu(net, sk-sk_bound_dev_if);
+ if (dev)
+ tb_id = vrf_dev_table_rcu(dev);
+ rcu_read_unlock();
+ }
+ chk_addr_ret = inet_addr_type_table(net, addr-sin_addr.s_addr, tb_id);
/* Not specified by any standard per-se, however it breaks too
* many applications when removed. It is unfortunate since
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 34a308573f4b..30409b75e925 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -233,7 +233,7 @@ static int arp_constructor(struct neighbour *neigh)
return -EINVAL;
}
- neigh-type = inet_addr_type(dev_net(dev), addr);
+ neigh-type = inet_addr_type_dev_table(dev_net(dev), dev, addr);
parms = in_dev-arp_parms;
__neigh_parms_put(neigh-parms);
@@ -343,7 +343,7 @@ static void arp_solicit(struct neighbour *neigh, struct
sk_buff *skb)
switch (IN_DEV_ARP_ANNOUNCE(in_dev)) {
default:
case 0: /* By default announce any local IP */
- if (skb inet_addr_type(dev_net(dev),
+ if (skb inet_addr_type_dev_table(dev_net(dev), dev,
ip_hdr(skb)-saddr) == RTN_LOCAL)
saddr = ip_hdr(skb)-saddr;
break;
@@ -351,7 +351,8 @@ static void arp_solicit(struct neighbour *neigh, struct
sk_buff *skb)
if (!skb)
break;
saddr = ip_hdr(skb)-saddr;
- if (inet_addr_type(dev_net(dev), saddr) == RTN_LOCAL) {
+ if (inet_addr_type_dev_table(dev_net(dev), dev,
+saddr) == RTN_LOCAL) {
/* saddr should be known to target */
if (inet_addr_onlink(in_dev, target, saddr))
break;
@@ -751,7 +752,7 @@ static int arp_process(struct sock *sk, struct sk_buff *skb)
/* Special case: IPv4 duplicate address detection packet (RFC2131) */
if (sip == 0) {
if (arp-ar_op == htons(ARPOP_REQUEST)
- inet_addr_type(net, tip) == RTN_LOCAL
+ inet_addr_type_dev_table(net, dev, tip) == RTN_LOCAL
!arp_ignore(in_dev, sip, tip))
arp_send(ARPOP_REPLY, ETH_P_ARP, sip, dev, tip,
