Re: [PATCH net] ipv6: avoid atomic fragment on GSO packets

From: Florian Westphal
Date: Mon Oct 02 2023 - 13:11:58 EST


Yan Zhai <yan@xxxxxxxxxxxxxx> wrote:
> On Sat, Sep 30, 2023 at 6:09 AM Florian Westphal <fw@xxxxxxxxx> wrote:
> >
> > This helper is also called for skbs where IP6CB(skb)->frag_max_size
> > exceeds the MTU, so this check looks wrong to me.
> >
> > Same remark for dst_allfrag() check in __ip6_finish_output(),
> > after this patch, it would be ignored.
> >
> Thanks for covering my carelessness. I was just considering the GSO
> case so frag_max_size was overlooked. dst_allfrag is indeed a case
> based on the code logic. But just out of curiosity, do we still see
> any use of this feature? From commit messages it is set when PMTU
> values signals smaller than min IPv6 MTU. But such PMTU values are
> just dropped in __ip6_rt_update_pmtu now. Iproute2 code also does not
> provide this route feature anymore. So it might be actually a dead
> check?

I don't think iproute2 ever exposed it, I think we can axe
dst_allfrag().

> > I think you should consider to first refactor __ip6_finish_output to make
> > the existing checks more readable (e.g. handle gso vs. non-gso in separate
> > branches) and then add the check to last seg in
> > ip6_finish_output_gso_slowpath_drop().
> >
> Agree with refactoring to mirror what IPv4 code is doing. It might not
> hurt if we check every segments in this case, since it is already the
> slowpath and it will make code more compact.

No objections from my side.