On 11/20/20 6:15 AM, Carlo Carraro wrote:
> I report here the issue with the previous patch.
> The code is now checking against params->tot_len but then it is still
> using is_skb_forwardable.
> Consider this case where I shrink the packet:
> skb->len == 1520
> dev->mtu == 1500
> params->tot_len == 1480
> So the incoming pkt has len 1520, and the out interface has mtu 1500.
> In this case fragmentation is not needed because params->tot_len < dev->mtu.
> However the code calls is_skb_forwardable and may return false because
> skb->len > dev->mtu, resulting in BPF_FIB_LKUP_RET_FRAG_NEEDED.
> What I propose is using params->tot_len only if provided, without
> falling back to use is_skb_forwardable when provided.
> Something like this:
>
> if (params->tot_len > 0) {
> if (params->tot_len > mtu)
> rc = BPF_FIB_LKUP_RET_FRAG_NEEDED;
> } else if (!is_skb_forwardable(dev, skb)) {
> rc = BPF_FIB_LKUP_RET_FRAG_NEEDED;
> }
>
> However, doing so we are skipping more relaxed MTU checks inside
> is_skb_forwardable, so I'm not sure about this.
> Please comment
Daniel's just proposed patch changes this again (removes the
is_skb_forwardable check). Jesper: you might want to hold off until that
happens.
