* VRF destination unreachable
@ 2018-02-23 17:49 Stephen Suryaputra
2018-02-23 20:58 ` David Ahern
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Suryaputra @ 2018-02-23 17:49 UTC (permalink / raw)
To: netdev
Greetings,
We found that ICMP destination unreachable isn't sent if VRF
forwarding isn't configured, i.e.
/proc/sys/net/ipv4/conf/<vrf_net_device>/forwarding isn't set. The
relevant code is:
static int ip_error(struct sk_buff *skb)
{
...
// in_dev is the vrf net_device
if (!IN_DEV_FORWARD(in_dev)) {
switch (rt->dst.error) {
case EHOSTUNREACH:
__IP_INC_STATS(net, IPSTATS_MIB_INADDRERRORS);
break;
case ENETUNREACH:
__IP_INC_STATS(net, IPSTATS_MIB_INNOROUTES);
break;
}
goto out;
}
...
out: kfree_skb(skb);
return 0;
}
The question: is it intended to be set? The basic forwarding seems to
be working without. We do set it on the slave net devices.
Thank you,
Stephen.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: VRF destination unreachable
2018-02-23 17:49 VRF destination unreachable Stephen Suryaputra
@ 2018-02-23 20:58 ` David Ahern
2018-02-27 16:09 ` Stephen Suryaputra
0 siblings, 1 reply; 4+ messages in thread
From: David Ahern @ 2018-02-23 20:58 UTC (permalink / raw)
To: Stephen Suryaputra, netdev
On 2/23/18 10:49 AM, Stephen Suryaputra wrote:
> Greetings,
>
> We found that ICMP destination unreachable isn't sent if VRF
> forwarding isn't configured, i.e.
> /proc/sys/net/ipv4/conf/<vrf_net_device>/forwarding isn't set. The
> relevant code is:
>
> static int ip_error(struct sk_buff *skb)
> {
> ...
> // in_dev is the vrf net_device
> if (!IN_DEV_FORWARD(in_dev)) {
> switch (rt->dst.error) {
> case EHOSTUNREACH:
> __IP_INC_STATS(net, IPSTATS_MIB_INADDRERRORS);
> break;
>
> case ENETUNREACH:
> __IP_INC_STATS(net, IPSTATS_MIB_INNOROUTES);
> break;
> }
> goto out;
> }
> ...
> out: kfree_skb(skb);
> return 0;
> }
>
> The question: is it intended to be set? The basic forwarding seems to
> be working without. We do set it on the slave net devices.
Unintended side effect of VRF as a netdev. This should fix it:
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 5ca7415cd48c..d59d005fb7c5 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -944,7 +944,7 @@ static int ip_error(struct sk_buff *skb)
goto out;
net = dev_net(rt->dst.dev);
- if (!IN_DEV_FORWARD(in_dev)) {
+ if (!IN_DEV_FORWARD(in_dev) && !netif_is_l3_master(skb->dev)) {
switch (rt->dst.error) {
case EHOSTUNREACH:
__IP_INC_STATS(net, IPSTATS_MIB_INADDRERRORS);
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: VRF destination unreachable
2018-02-23 20:58 ` David Ahern
@ 2018-02-27 16:09 ` Stephen Suryaputra
2018-02-27 17:09 ` David Ahern
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Suryaputra @ 2018-02-27 16:09 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
Thanks David for confirming that this is unintended.
Given that, I think the fix isn't complete because when forwarding
isn't enabled on the original incoming netdev, the ICMP shouldn't be
generated. This diff fixes that case:
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a4f44d8..dc40a94 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -937,13 +937,20 @@ static int ip_error(struct sk_buff *skb)
struct net *net;
bool send;
int code;
+ int in_dev_forward;
/* IP on this device is disabled. */
if (!in_dev)
goto out;
net = dev_net(rt->dst.dev);
- if (!IN_DEV_FORWARD(in_dev)) {
+ if (netif_is_l3_master(skb->dev)) {
+ in_dev_forward = IN_DEV_FORWARD(
+ __in_dev_get_rcu(__dev_get_by_index(net,
IPCB(skb)->iif)));
+ } else {
+ in_dev_forward = IN_DEV_FORWARD(in_dev);
+ }
+ if (!in_dev_forward) {
switch (rt->dst.error) {
case EHOSTUNREACH:
__IP_INC_STATS(net, IPSTATS_MIB_INADDRERRORS);
I can put a formal patch if you like.
Regards,
Stephen.
On Fri, Feb 23, 2018 at 3:58 PM, David Ahern <dsahern@gmail.com> wrote:
> On 2/23/18 10:49 AM, Stephen Suryaputra wrote:
>> Greetings,
>>
>> We found that ICMP destination unreachable isn't sent if VRF
>> forwarding isn't configured, i.e.
>> /proc/sys/net/ipv4/conf/<vrf_net_device>/forwarding isn't set. The
>> relevant code is:
>>
>> static int ip_error(struct sk_buff *skb)
>> {
>> ...
>> // in_dev is the vrf net_device
>> if (!IN_DEV_FORWARD(in_dev)) {
>> switch (rt->dst.error) {
>> case EHOSTUNREACH:
>> __IP_INC_STATS(net, IPSTATS_MIB_INADDRERRORS);
>> break;
>>
>> case ENETUNREACH:
>> __IP_INC_STATS(net, IPSTATS_MIB_INNOROUTES);
>> break;
>> }
>> goto out;
>> }
>> ...
>> out: kfree_skb(skb);
>> return 0;
>> }
>>
>> The question: is it intended to be set? The basic forwarding seems to
>> be working without. We do set it on the slave net devices.
>
> Unintended side effect of VRF as a netdev. This should fix it:
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 5ca7415cd48c..d59d005fb7c5 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -944,7 +944,7 @@ static int ip_error(struct sk_buff *skb)
> goto out;
>
> net = dev_net(rt->dst.dev);
> - if (!IN_DEV_FORWARD(in_dev)) {
> + if (!IN_DEV_FORWARD(in_dev) && !netif_is_l3_master(skb->dev)) {
> switch (rt->dst.error) {
> case EHOSTUNREACH:
> __IP_INC_STATS(net, IPSTATS_MIB_INADDRERRORS);
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: VRF destination unreachable
2018-02-27 16:09 ` Stephen Suryaputra
@ 2018-02-27 17:09 ` David Ahern
0 siblings, 0 replies; 4+ messages in thread
From: David Ahern @ 2018-02-27 17:09 UTC (permalink / raw)
To: Stephen Suryaputra; +Cc: netdev
On 2/27/18 9:09 AM, Stephen Suryaputra wrote:
> Thanks David for confirming that this is unintended.
>
> Given that, I think the fix isn't complete because when forwarding
> isn't enabled on the original incoming netdev, the ICMP shouldn't be
> generated. This diff fixes that case:
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index a4f44d8..dc40a94 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -937,13 +937,20 @@ static int ip_error(struct sk_buff *skb)
> struct net *net;
> bool send;
> int code;
> + int in_dev_forward;
declarations are in reverse xmas tree order. ie., put this new one after
peer.
>
> /* IP on this device is disabled. */
> if (!in_dev)
> goto out;
>
> net = dev_net(rt->dst.dev);
> - if (!IN_DEV_FORWARD(in_dev)) {
> + if (netif_is_l3_master(skb->dev)) {
> + in_dev_forward = IN_DEV_FORWARD(
> + __in_dev_get_rcu(__dev_get_by_index(net,
> IPCB(skb)->iif)));
> + } else {
> + in_dev_forward = IN_DEV_FORWARD(in_dev);
> + }
That looks correct.
> + if (!in_dev_forward) {
> switch (rt->dst.error) {
> case EHOSTUNREACH:
> __IP_INC_STATS(net, IPSTATS_MIB_INADDRERRORS);
>
> I can put a formal patch if you like.
>
please do.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-02-27 17:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-23 17:49 VRF destination unreachable Stephen Suryaputra
2018-02-23 20:58 ` David Ahern
2018-02-27 16:09 ` Stephen Suryaputra
2018-02-27 17:09 ` David Ahern
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).