netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] ipvlan: read direct ifindex instead of iflink
@ 2015-10-20 23:47 Brenden Blanco
  2015-10-21 11:26 ` Jiri Benc
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Brenden Blanco @ 2015-10-20 23:47 UTC (permalink / raw)
  To: davem; +Cc: netdev, maheshb

In the ipv4 outbound path of an ipvlan device in l3 mode, the ifindex is
being grabbed from dev_get_iflink. This works for the physical device
case, since as the documentation of that function notes: "Physical
interfaces have the same 'ifindex' and 'iflink' values.".  However, if
the master device is a veth, and the pairs are in separate net
namespaces, the route lookup will fail with -ENODEV due to outer veth
pair being in a separate namespace from the ipvlan master/routing
namespace.

  ns0    |   ns1    |   ns2
 veth0a--|--veth0b--|--ipvl0

In ipvlan_process_v4_outbound(), a packet sent from ipvl0 in the above
configuration will pass fl.flowi4_oif == veth0a to
ip_route_output_flow(), but *net == ns1.

Notice also that ipv6 processing is not using iflink. Since there is a
discrepancy in usage, fixup both v4 and v6 case to use local dev
variable.

Tested this with l3 ipvlan on top of veth, as well as with single
physical interface in the top namespace.

Signed-off-by: Brenden Blanco <bblanco@plumgrid.com>
---
 drivers/net/ipvlan/ipvlan_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 24f8dbc..d50887e 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -348,7 +348,7 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
 	struct rtable *rt;
 	int err, ret = NET_XMIT_DROP;
 	struct flowi4 fl4 = {
-		.flowi4_oif = dev_get_iflink(dev),
+		.flowi4_oif = dev->ifindex,
 		.flowi4_tos = RT_TOS(ip4h->tos),
 		.flowi4_flags = FLOWI_FLAG_ANYSRC,
 		.daddr = ip4h->daddr,
@@ -386,7 +386,7 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
 	struct dst_entry *dst;
 	int err, ret = NET_XMIT_DROP;
 	struct flowi6 fl6 = {
-		.flowi6_iif = skb->dev->ifindex,
+		.flowi6_iif = dev->ifindex,
 		.daddr = ip6h->daddr,
 		.saddr = ip6h->saddr,
 		.flowi6_flags = FLOWI_FLAG_ANYSRC,
-- 
2.6.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] ipvlan: read direct ifindex instead of iflink
  2015-10-20 23:47 [PATCH net-next] ipvlan: read direct ifindex instead of iflink Brenden Blanco
@ 2015-10-21 11:26 ` Jiri Benc
  2015-10-21 18:32 ` Mahesh Bandewar
  2015-10-22 13:39 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Jiri Benc @ 2015-10-21 11:26 UTC (permalink / raw)
  To: Brenden Blanco; +Cc: davem, netdev, maheshb

On Tue, 20 Oct 2015 16:47:33 -0700, Brenden Blanco wrote:
> --- a/drivers/net/ipvlan/ipvlan_core.c
> +++ b/drivers/net/ipvlan/ipvlan_core.c
> @@ -348,7 +348,7 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
>  	struct rtable *rt;
>  	int err, ret = NET_XMIT_DROP;
>  	struct flowi4 fl4 = {
> -		.flowi4_oif = dev_get_iflink(dev),
> +		.flowi4_oif = dev->ifindex,

The current code is obviously wrong and the patch makes sense.

Reviewed-by: Jiri Benc <jbenc@redhat.com>

-- 
Jiri Benc

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] ipvlan: read direct ifindex instead of iflink
  2015-10-20 23:47 [PATCH net-next] ipvlan: read direct ifindex instead of iflink Brenden Blanco
  2015-10-21 11:26 ` Jiri Benc
@ 2015-10-21 18:32 ` Mahesh Bandewar
  2015-10-22 13:39 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Mahesh Bandewar @ 2015-10-21 18:32 UTC (permalink / raw)
  To: Brenden Blanco; +Cc: David Miller, linux-netdev

On Tue, Oct 20, 2015 at 4:47 PM, Brenden Blanco <bblanco@plumgrid.com> wrote:
> In the ipv4 outbound path of an ipvlan device in l3 mode, the ifindex is
> being grabbed from dev_get_iflink. This works for the physical device
> case, since as the documentation of that function notes: "Physical
> interfaces have the same 'ifindex' and 'iflink' values.".  However, if
> the master device is a veth, and the pairs are in separate net
> namespaces, the route lookup will fail with -ENODEV due to outer veth
> pair being in a separate namespace from the ipvlan master/routing
> namespace.
>
>   ns0    |   ns1    |   ns2
>  veth0a--|--veth0b--|--ipvl0
>
interesting use case.

> In ipvlan_process_v4_outbound(), a packet sent from ipvl0 in the above
> configuration will pass fl.flowi4_oif == veth0a to
> ip_route_output_flow(), but *net == ns1.
>
> Notice also that ipv6 processing is not using iflink. Since there is a
> discrepancy in usage, fixup both v4 and v6 case to use local dev
> variable.
>
> Tested this with l3 ipvlan on top of veth, as well as with single
> physical interface in the top namespace.
>
> Signed-off-by: Brenden Blanco <bblanco@plumgrid.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  drivers/net/ipvlan/ipvlan_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
> index 24f8dbc..d50887e 100644
> --- a/drivers/net/ipvlan/ipvlan_core.c
> +++ b/drivers/net/ipvlan/ipvlan_core.c
> @@ -348,7 +348,7 @@ static int ipvlan_process_v4_outbound(struct sk_buff *skb)
>         struct rtable *rt;
>         int err, ret = NET_XMIT_DROP;
>         struct flowi4 fl4 = {
> -               .flowi4_oif = dev_get_iflink(dev),
> +               .flowi4_oif = dev->ifindex,
>                 .flowi4_tos = RT_TOS(ip4h->tos),
>                 .flowi4_flags = FLOWI_FLAG_ANYSRC,
>                 .daddr = ip4h->daddr,
> @@ -386,7 +386,7 @@ static int ipvlan_process_v6_outbound(struct sk_buff *skb)
>         struct dst_entry *dst;
>         int err, ret = NET_XMIT_DROP;
>         struct flowi6 fl6 = {
> -               .flowi6_iif = skb->dev->ifindex,
> +               .flowi6_iif = dev->ifindex,
>                 .daddr = ip6h->daddr,
>                 .saddr = ip6h->saddr,
>                 .flowi6_flags = FLOWI_FLAG_ANYSRC,
> --
> 2.6.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net-next] ipvlan: read direct ifindex instead of iflink
  2015-10-20 23:47 [PATCH net-next] ipvlan: read direct ifindex instead of iflink Brenden Blanco
  2015-10-21 11:26 ` Jiri Benc
  2015-10-21 18:32 ` Mahesh Bandewar
@ 2015-10-22 13:39 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2015-10-22 13:39 UTC (permalink / raw)
  To: bblanco; +Cc: netdev, maheshb

From: Brenden Blanco <bblanco@plumgrid.com>
Date: Tue, 20 Oct 2015 16:47:33 -0700

> In the ipv4 outbound path of an ipvlan device in l3 mode, the ifindex is
> being grabbed from dev_get_iflink. This works for the physical device
> case, since as the documentation of that function notes: "Physical
> interfaces have the same 'ifindex' and 'iflink' values.".  However, if
> the master device is a veth, and the pairs are in separate net
> namespaces, the route lookup will fail with -ENODEV due to outer veth
> pair being in a separate namespace from the ipvlan master/routing
> namespace.
> 
>   ns0    |   ns1    |   ns2
>  veth0a--|--veth0b--|--ipvl0
> 
> In ipvlan_process_v4_outbound(), a packet sent from ipvl0 in the above
> configuration will pass fl.flowi4_oif == veth0a to
> ip_route_output_flow(), but *net == ns1.
> 
> Notice also that ipv6 processing is not using iflink. Since there is a
> discrepancy in usage, fixup both v4 and v6 case to use local dev
> variable.
> 
> Tested this with l3 ipvlan on top of veth, as well as with single
> physical interface in the top namespace.
> 
> Signed-off-by: Brenden Blanco <bblanco@plumgrid.com>

Applied, thanks Brenden.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-10-22 13:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-20 23:47 [PATCH net-next] ipvlan: read direct ifindex instead of iflink Brenden Blanco
2015-10-21 11:26 ` Jiri Benc
2015-10-21 18:32 ` Mahesh Bandewar
2015-10-22 13:39 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).