Re: [PATCH net-next v3 0/3] virtio_net: add optional flow dissection in virtio_net_hdr_to_skb
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: 2021-06-04 03:52:09
On Thu, Jun 3, 2021 at 10:55 PM Jason Wang [off-list ref] wrote:
在 2021/6/2 上午6:18, Tanner Love 写道:quoted
From: Tanner Love <redacted> First patch extends the flow dissector BPF program type to accept virtio-net header members. Second patch uses this feature to add optional flow dissection in virtio_net_hdr_to_skb(). This allows admins to define permitted packets more strictly, for example dropping deprecated UDP_UFO packets. Third patch extends kselftest to cover this feature.I wonder why virtio maintainers is not copied in this series.
Sorry, an oversight.
Several questions: 1) having bpf core to know about virito-net header seems like a layer violation, it doesn't scale as we may add new fields, actually there's already fields that is not implemented in the spec but not Linux right now.
struct virtio_net_hdr is used by multiple interfaces, not just virtio. The interface as is will remain, regardless of additional extensions. If the interface is extended, the validation can be extended with it. Just curious: can you share what extra fields may be in the pipeline? The struct has historically not seen (m)any changes.
2) virtio_net_hdr_to_skb() is not the single entry point, packet could go via XDP
Do you mean AF_XDP? As far as I know, vnet_hdr is the only injection interface for complex packets that include offload instructions (GSO, csum) -- which are the ones mostly implicated in bug reports.
3) I wonder whether we can simply use XDP to solve this issue (metadata probably but I don't have a deep thought) 4) If I understand the code correctly, it should deal with all dodgy packets instead of just for virtio
Yes. Some callers of virtio_net_hdr_to_skb, such as tun_get_user and virtio receive_buf, pass all packets to it. Others, like tap_get_user and packet_snd, only call it if a virtio_net_hdr is passed. Once we have a validation hook, ideally all packets need to pass it. Modifying callers like tap_get_user can be a simple follow-on.