On 1/13/26 3:39 PM, Michael S. Tsirkin wrote:
> On Tue, Jan 13, 2026 at 03:30:00PM +0100, Paolo Abeni wrote:
>> On 1/10/26 9:07 AM, Gustavo A. R. Silva wrote:
>>> Use the new TRAILING_OVERLAP() helper to fix a misalignment bug
>>> along with the following warning:
>>>
>>> drivers/net/virtio_net.c:429:46: warning: structure containing a flexible 
>>> array member is not at the end of another structure 
>>> [-Wflex-array-member-not-at-end]
>>>
>>> This helper creates a union between a flexible-array member (FAM)
>>> and a set of members that would otherwise follow it (in this case
>>> `u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];`). This
>>> overlays the trailing members (rss_hash_key_data) onto the FAM
>>> (hash_key_data) while keeping the FAM and the start of MEMBERS aligned.
>>> The static_assert() ensures this alignment remains.
>>>
>>> Notice that due to tail padding in flexible `struct
>>> virtio_net_rss_config_trailer`, `rss_trailer.hash_key_data`
>>> (at offset 83 in struct virtnet_info) and `rss_hash_key_data` (at
>>> offset 84 in struct virtnet_info) are misaligned by one byte. See
>>> below:
>>>
>>> struct virtio_net_rss_config_trailer {
>>>         __le16                     max_tx_vq;            /*     0     2 */
>>>         __u8                       hash_key_length;      /*     2     1 */
>>>         __u8                       hash_key_data[];      /*     3     0 */
>>>
>>>         /* size: 4, cachelines: 1, members: 3 */
>>>         /* padding: 1 */
>>>         /* last cacheline: 4 bytes */
>>> };
>>>
>>> struct virtnet_info {
>>> ...
>>>         struct virtio_net_rss_config_trailer rss_trailer; /*    80     4 */
>>>
>>>         /* XXX last struct has 1 byte of padding */
>>>
>>>         u8                         rss_hash_key_data[40]; /*    84    40 */
>>> ...
>>>         /* size: 832, cachelines: 13, members: 48 */
>>>         /* sum members: 801, holes: 8, sum holes: 31 */
>>>         /* paddings: 2, sum paddings: 5 */
>>> };
>>>
>>> After changes, those members are correctly aligned at offset 795:
>>>
>>> struct virtnet_info {
>>> ...
>>>         union {
>>>                 struct virtio_net_rss_config_trailer rss_trailer; /*   792  
>>>    4 */
>>>                 struct {
>>>                         unsigned char __offset_to_hash_key_data[3]; /*   
>>> 792     3 */
>>>                         u8         rss_hash_key_data[40]; /*   795    40 */
>>>                 };                                       /*   792    43 */
>>>         };                                               /*   792    44 */
>>> ...
>>>         /* size: 840, cachelines: 14, members: 47 */
>>>         /* sum members: 801, holes: 8, sum holes: 35 */
>>>         /* padding: 4 */
>>>         /* paddings: 1, sum paddings: 4 */
>>>         /* last cacheline: 8 bytes */
>>> };
>>>
>>> As a result, the RSS key passed to the device is shifted by 1
>>> byte: the last byte is cut off, and instead a (possibly
>>> uninitialized) byte is added at the beginning.
>>>
>>> As a last note `struct virtio_net_rss_config_hdr *rss_hdr;` is also
>>> moved to the end, since it seems those three members should stick
>>> around together. :)
>>>
>>> Cc: [email protected]
>>> Fixes: ed3100e90d0d ("virtio_net: Use new RSS config structs")
>>> Signed-off-by: Gustavo A. R. Silva <[email protected]>
>>> ---
>>> Changes in v2:
>>>  - Update subject and changelog text (include feedback from Simon and
>>>    Michael --thanks folks)
>>>  - Add Fixes tag and CC -stable.
>>
>> @Michael, @Jason: This is still apparently targeting 'net-next', but I
>> think it should land in the 'net' tree, right?
>>
>> /P
> 
> Probably but I'm yet to properly review it. The thing that puzzles me at
> a first glance is how are things working right now then?

Apparently they aren't ?!?

rss self-tests for virtio_net are failing:

https://netdev-ctrl.bots.linux.dev/logs/vmksft/drv-hw-dbg/results/471521/15-rss-api-py/stdout

but the result is into the CI reported as success (no idea why?!?)

/P


Reply via email to