From: David Ahern <dsahern@gmail•com>
To: Stephen Suryaputra <ssuryaextr@gmail•com>,
netdev@vger•kernel.org, ebiederm@xmission•com, roopa@nvidia•com
Cc: fw@strlen•de
Subject: Re: IPCB isn't initialized when MPLS route is pointing to a VRF
Date: Sun, 28 Nov 2021 15:48:32 -0700 [thread overview]
Message-ID: <1b3a135b-e4f4-dec8-de40-09d135a3f7eb@gmail.com> (raw)
In-Reply-To: <20211122160546.GA95191@ssuryadesk>
On 11/22/21 9:05 AM, Stephen Suryaputra wrote:
> Hi,
>
> We ran into a problem because IPCB isn't being initialized when MPLS is
> egressing into a VRF. Reproducer script and its teardown are attached,
> but they are only to illustrate our setup rather than seeing the problem
> as it depends on what is in the skb->cb[].
>
> We found this when h0 is sending an ip packet with DF=1 to 10.1.4.2 and
> on ler1 the code path is as follows: mpls_forward() calls mpls_egress()
> and then calls neigh_xmit(), which ends up calling __dev_queue_xmit()
> and vrf_xmit() through dev_hard_start_xmit(). vrf_xmit() eventually will
> call ip_output() and __ip_finish_output() that has the check for
> IPCB(skb)->frag_max_size. The conditional happens to be true for us due
> to IPCB isn't being initialized even though the packet size is small.
> The packet then is dropped in ip_fragment().
>
> The change in this diff is a way to fix:
>
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index ffeb2df8be7a..1d0a0151e175 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -310,6 +310,7 @@ static bool mpls_egress(struct net *net, struct mpls_route *rt,
> htons(hdr4->ttl << 8),
> htons(new_ttl << 8));
> hdr4->ttl = new_ttl;
> + memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
> success = true;
> break;
> }
> @@ -327,6 +328,7 @@ static bool mpls_egress(struct net *net, struct mpls_route *rt,
> hdr6->hop_limit = dec.ttl;
> else if (hdr6->hop_limit)
> hdr6->hop_limit = hdr6->hop_limit - 1;
> + memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
> success = true;
> break;
> }
>
> Here are my questions. Is this the best place to initialize the IPCB?
> Would it be better done in vrf.c? I can work on the formal patch once we
> agree on where the final fix should be
vrf.c seems like the right place since it does the direct recirculation
(vs loopback which puts in back in the Rx queue).
>
> Cc Florian Westphal since we found the problem after upgrading to kernel
> version that has his commit bb4cc1a18856 ("net: ip: always refragment ip
> defragmented packets"). But I think the bug is there without his commit
> as well if (IPCB(skb)->flags & IPCB_FRAG_PMTU) happens to evaluate to
> true.
>
> Thanks,
>
> Stephen.
>
prev parent reply other threads:[~2021-11-28 22:51 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-22 16:05 IPCB isn't initialized when MPLS route is pointing to a VRF Stephen Suryaputra
2021-11-22 16:07 ` Stephen Suryaputra
2021-11-28 22:48 ` David Ahern [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1b3a135b-e4f4-dec8-de40-09d135a3f7eb@gmail.com \
--to=dsahern@gmail$(echo .)com \
--cc=ebiederm@xmission$(echo .)com \
--cc=fw@strlen$(echo .)de \
--cc=netdev@vger$(echo .)kernel.org \
--cc=roopa@nvidia$(echo .)com \
--cc=ssuryaextr@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox