RE: [PATCH] net: pad virtio_net_ff_cap_data

Parav Pandit <[email protected]> Wed, 19 Nov 2025 09:38:18 +0000
Newsgroups dev.linux.lists.virtio-comment
Message-ID <CY8PR12MB7195E4A973474CAF2A68E001DCD7A@CY8PR12MB7195.namprd12.prod.outlook.com>

> From: Michael S. Tsirkin <[email protected]>
> Sent: 19 November 2025 02:04 PM
> 
> On Wed, Nov 19, 2025 at 08:29:48AM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <[email protected]>
> > > Sent: 19 November 2025 01:54 PM
> > >
> > > struct virtio_net_ff_cap_data has 4 byte fields but the size is not
> > > a multiple of 4.  drivers can easily get it wrong since compilers
> > > tend to add padding to align such structures.
> > >
> > > Since we are always allowed to pad or truncate admin commands, let's
> > > do just that here.
> > >
> > > Fixes: 899bb0c ("virtio-net: Add flow filter capability")
Prefer 12 letter tag.
Not a must as we are about to release.
So will change locally when I merge.

> > > Cc: "Parav Pandit" <[email protected]>
> > > Signed-off-by: Michael S. Tsirkin <[email protected]>
> > > ---
> > >  device-types/net/description.tex | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/device-types/net/description.tex b/device-
> > > types/net/description.tex index 9833d16..0b4fcac 100644
> > > --- a/device-types/net/description.tex
> > > +++ b/device-types/net/description.tex
> > > @@ -3055,6 +3055,7 @@ \subsubsection{Flow filter}\label{sec:Device
> > > Types / Network Device / Device Ope
> > >          le32 rules_per_group_limit;
> > >          u8 last_rule_priority;
> > >          u8 selectors_per_classifier_limit;
> > > +        u8 reserved[2];
> > >  };
> > >  \end{lstlisting}
> > >
> > Thanks for the fix.
> > We should align to the 8B boundary instead of 4B.
> > It should be reserved[6].
> 
> 
> I prefer the 4b one because it likely is what drivers have been doing anyway,
> unknowingly.
>
Ok.
 
> > > @@ -3071,6 +3072,8 @@ \subsubsection{Flow filter}\label{sec:Device
> > > Types / Network Device / Device Ope
> > > \field{selectors_per_classifier_limit} is the maximum number of selectors
> that a classifier can have.
> > >
> > > +\field{reserved} is reserved and set to zero.
> > > +
> > >  \subparagraph{VIRTIO_NET_FF_SELECTOR_CAP}
> > >  \label{par:Device Types / Network Device / Device Operation / Flow
> > > filter / Device and driver capabilities /
> > > VIRTIO-NET-FF-SELECTOR-CAP}
> > >
> > > --
> > > MST
Reviewed-by: Parav Pandit <[email protected]>