On Mon, 3 Aug 2020 17:44:16 -0600
David Ahern <dsah...@gmail.com> wrote:
> On 8/3/20 2:52 PM, Stefano Brivio wrote:
> > @@ -461,6 +464,91 @@ static inline void iptunnel_xmit_stats(struct
> > net_device *dev, int pkt_len)
> > [...]
> >
> > +static inline int skb_tunnel_check_pmtu(struct sk_buff *skb,
> > + struct dst_entry *encap_dst,
> > + int headroom, bool reply)
>
> Given its size, this is probably better as a function. I believe it can
> go into net/ipv4/ip_tunnel_core.c like you have iptunnel_pmtud_build_icmp.
Right, moved in v2.
> > [...]
> > + if (skb->protocol == htons(ETH_P_IP) && mtu > 576) {
>
> I am surprised the 576 does not have an existing macro.
I guess that comes from how RFC 791 picks this "512 plus something
reasonable" value. I'll think of a name and propose as a later patch,
it's used in a number of places.
> > [...]
> > + return iptunnel_pmtud_build_icmp(skb, mtu);
> > + }
> > +#endif
>
> separate v4 and v6 code into helpers based on skb->protocol; the mtu
> check then becomes part of the version specific helpers.
Done.
> > +EXPORT_SYMBOL(iptunnel_pmtud_build_icmp);
>
> I think separate v4 and v6 versions would be more readable; the
> duplication is mostly skb manipulation.
Yes, way more readable, changed in v2.
--
Stefano
