On Tue, Apr 19, 2022 at 09:56:02AM -0400, Willem de Bruijn wrote: > > > > > > We should also maintain feature consistency between packet_snd, > > > tpacket_snd and to the limitations of its feature set to > > > packet_sendmsg_spkt. The no_fcs is already lacking in tpacket_snd as > > > far as I can tell. But packet_sendmsg_spkt also sets it and calls > > > packet_parse_headers. > > > > Yes, I think we could fix the tpacket_snd() in another patch. > > > > There are also some duplicated codes in these *_snd functions. > > I think we can move them out to one single function. > > Please don't refactor this code. It will complicate future backports > of stable fixes. Hmm I don't know offhand which duplication this refers to specifically so maybe it's not worth addressing specifically but generally not cleaning up code just because of backports seems wrong ... > > > Because this patch touches many other packets besides the ones > > > intended, I am a bit concerned about unintended consequences. Perhaps > > > > Yes, makes sense. > > > > > stretching the definition of the flags to include VLAN is acceptable > > > (unlike outright tunnels), but even then I would suggest for net-next. > > > > As I asked, I'm not familiar with virtio code. Do you think if I should > > add a new VIRTIO_NET_HDR_GSO_VLAN flag? It's only a L2 flag without any L3 > > info. If I add something like VIRTIO_NET_HDR_GSO_VLAN_TCPV4/TCPV6/UDP. That > > would add more combinations. Which doesn't like a good idea. > > I would prefer a new flag to denote this type, so that we can be > strict and only change the datapath for packets that have this flag > set (and thus express the intent). > > But the VIRTIO_NET_HDR types are defined in the virtio spec. The > maintainers should probably chime in. Yes, it's a UAPI extension, not to be done lightly. In this case IIUC gso_type in the header is only u8 - 8 bits and 5 of these are already used. So I don't think the virtio TC will be all that happy to burn up a bit unless a clear benefit can be demonstrated. I agree with the net-next proposal, I think it's more a feature than a bugfix. In particular I think a Fixes tag can also be dropped in that IIUC GSO for vlan packets didn't work even before that commit - right? -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
