netdev - Re: [PATCH 2/2] net/packet: send and receive pkt with given vnet_hdr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <63f62f125ac3b_2ab6208b4@willemb.c.googlers.com.notmuch>
Date:   Wed, 22 Feb 2023 10:04:50 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     沈安琪(凛玥) <amy.saq@...group.com>,
        "Michael S. Tsirkin" <mst@...hat.com>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        netdev@...r.kernel.org, davem@...emloft.net, jasowang@...hat.com,
        谈鉴锋 <henry.tjf@...group.com>
Subject: Re: [PATCH 2/2] net/packet: send and receive pkt with given
 vnet_hdr_sz

> >>> You're right. This is configured from userspace before the FD is passed
> >>> to vhost-net, so indeed this will require packet socket UAPI support.
> >>
> >> Thanks for quick reply. We will go with adding an extra UAPI here then.
> >>
> >>
> >> Another discussion for designing this UAPI is, whether it will be better to
> >> support setting only vnet header size, just like what TAP does in its ioctl,
> >> or to support setting a virtio feature bit-map.
> >>
> >>
> >> UAPI setting only vnet header size
> >>
> >> Pros:
> >>
> >> 1. It aligns with how other virito backend devices communicate with
> >> virtio-user
> >>
> >> 2. We can use the holes in struct packet_socket (net/packet/internal.h:120)
> >> to record the extra information since the size info only takes 8 bits.
> >>
> >> Cons:
> >>
> >> 1. It may have more information that virtio-user needs to communicate with
> >> packet socket in the future and needs to add more UAPI supports here.
> >>
> >> To Michael: Is there any other information that backend device needs and
> >> will be given from virtio-user?
> >
> > Yes e.g. I already mentioned virtio 1.0 wrt LE versus native endian
> > format.
> >
> >
> >> UAPI setting a virtio feature bit-map
> >>
> >> Pros:
> >>
> >> 1. It is more general and may reduce future UAPI changes.
> >>
> >> Cons:
> >>
> >> 1. A virtio feature bit-map needs 64 bits, which needs to add an extra field
> >> in packet_sock struct

Accepting a bitmap in the ABI does not have to imply storing a bitmap.

> >>
> >> 2. Virtio-user needs to aware that using packet socket as backend supports
> >> different approach to negotiate the vnet header size.
> >>
> >>
> >> We really appreciate any suggestion or discussion on this design choice of
> >> UAPI.
> > In the end it's ok with just size too, you just probably shouldn't say
> > you support VERSION_1 if you are not passing that bit.
> >
> 
> Sorry for the confusion here that we mentioned VERSION_1 in the commit 
> log. We actually just attended to give an example of what features that 
> may need 12-byte vnet header. We will remove it from the commit log in 
> patch v2 to avoid confusion here. Thanks a lot for your suggestions.

The question hinges on which features are expected to have to be
supported in the future. So far we have

- extra num_buffers field
- little endian (V1)

Given the rate of change in the spec, I don't think this should
be over designed. If V1 is not planned to be supported, just
configure header length. If it is, then perhaps instead a feature
bitmap.