* [PATCH] net: pad virtio_net_ff_cap_data
@ 2025-11-19 8:23 Michael S. Tsirkin
2025-11-19 8:29 ` Parav Pandit
0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 8:23 UTC (permalink / raw)
To: virtio-comment; +Cc: Parav Pandit
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")
Cc: "Parav Pandit" <parav@nvidia.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
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}
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: [PATCH] net: pad virtio_net_ff_cap_data
2025-11-19 8:23 [PATCH] net: pad virtio_net_ff_cap_data Michael S. Tsirkin
@ 2025-11-19 8:29 ` Parav Pandit
2025-11-19 8:33 ` Michael S. Tsirkin
2025-11-19 8:34 ` Michael S. Tsirkin
0 siblings, 2 replies; 6+ messages in thread
From: Parav Pandit @ 2025-11-19 8:29 UTC (permalink / raw)
To: Michael S. Tsirkin, virtio-comment@lists.linux.dev
> From: Michael S. Tsirkin <mst@redhat.com>
> 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")
> Cc: "Parav Pandit" <parav@nvidia.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 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].
> @@ -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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: pad virtio_net_ff_cap_data
2025-11-19 8:29 ` Parav Pandit
@ 2025-11-19 8:33 ` Michael S. Tsirkin
2025-11-19 8:34 ` Michael S. Tsirkin
1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 8:33 UTC (permalink / raw)
To: Parav Pandit; +Cc: virtio-comment@lists.linux.dev
On Wed, Nov 19, 2025 at 08:29:48AM +0000, Parav Pandit wrote:
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > 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")
> > Cc: "Parav Pandit" <parav@nvidia.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 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.
We can but we don't have to. We need to align on the
size of the largest field in the structure. This one
only has 4B fields.
> It should be reserved[6].
>
> > @@ -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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: pad virtio_net_ff_cap_data
2025-11-19 8:29 ` Parav Pandit
2025-11-19 8:33 ` Michael S. Tsirkin
@ 2025-11-19 8:34 ` Michael S. Tsirkin
2025-11-19 9:38 ` Parav Pandit
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 8:34 UTC (permalink / raw)
To: Parav Pandit; +Cc: virtio-comment@lists.linux.dev
On Wed, Nov 19, 2025 at 08:29:48AM +0000, Parav Pandit wrote:
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > 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")
> > Cc: "Parav Pandit" <parav@nvidia.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 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.
> > @@ -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
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] net: pad virtio_net_ff_cap_data
2025-11-19 8:34 ` Michael S. Tsirkin
@ 2025-11-19 9:38 ` Parav Pandit
2025-11-19 9:41 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Parav Pandit @ 2025-11-19 9:38 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-comment@lists.linux.dev
> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: 19 November 2025 02:04 PM
>
> On Wed, Nov 19, 2025 at 08:29:48AM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > 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" <parav@nvidia.com>
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > > 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 <parav@nvidia.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: pad virtio_net_ff_cap_data
2025-11-19 9:38 ` Parav Pandit
@ 2025-11-19 9:41 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2025-11-19 9:41 UTC (permalink / raw)
To: Parav Pandit; +Cc: virtio-comment@lists.linux.dev
On Wed, Nov 19, 2025 at 09:38:18AM +0000, Parav Pandit wrote:
>
>
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: 19 November 2025 02:04 PM
> >
> > On Wed, Nov 19, 2025 at 08:29:48AM +0000, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > 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.
thanks
> > > > Cc: "Parav Pandit" <parav@nvidia.com>
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > > 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 <parav@nvidia.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-19 9:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-19 8:23 [PATCH] net: pad virtio_net_ff_cap_data Michael S. Tsirkin
2025-11-19 8:29 ` Parav Pandit
2025-11-19 8:33 ` Michael S. Tsirkin
2025-11-19 8:34 ` Michael S. Tsirkin
2025-11-19 9:38 ` Parav Pandit
2025-11-19 9:41 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox