[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH v6 2/5] virtio-net: Add flow filter capabilities read commands
> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, November 22, 2023 7:32 PM
>
> On Fri, Nov 10, 2023 at 02:38:50PM +0200, Parav Pandit wrote:
> > The device responds flow filter capabilities using two commands.
> > One command indicates generic flow filter device limits such as number
> > of flow filters, number of flow filter groups, support or multiple
> > transports etc.
> >
> > The second command indicates supported match types, and fields of the
> > packet.
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/179
> > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> >
> > ---
> > changelog:
> > v2->v3:
> > - rebased on virtio-1.4 branch
> > - removed reference for flow filter virtqueue
> > v1->v2:
> > - addressed comments from Satananda
> > - added vlan type match field
> > - kept space for types between l2, l3, l4 header match types
> > - renamed mask to mask_supported with shorter width
> > - made more fields reserved for furture
> > - addressed comments from Heng
> > - grammar correction
> > - added field to indicate supported number of actions per flow
> > filter match entry
> > - added missing documentation for max_flow_priorities_per_group
> > v0->v1:
> > - added mask field in the type to indicate supported mask by device
> > and also in later patch to use it to indicate mask on adding
> > flow filter. As a result removed the mask_supported capability
> > field
> > ---
> > device-types/net/description.tex | 208
> > ++++++++++++++++++++++++++++++-
> > 1 file changed, 206 insertions(+), 2 deletions(-)
> >
> > diff --git a/device-types/net/description.tex
> > b/device-types/net/description.tex
> > index 30220b5..eccd8d6 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -1173,7 +1173,11 @@ \subsubsection{Flow Filter}\label{sec:Device
> > Types / Network Device / Device Ope
> >
> > The device indicates the flow filter capabilities to the driver.
> > These capabilities include various maximum device limits and
> > -supported packet match fields.
> > +supported packet match fields. These control virtqueue commands are:
> > +\ref{sec:Device Types / Network Device / Device Operation / Control
> > +Virtqueue / Flow Filter / Flow Filter Capabilities Get} and
> > +\ref{sec:Device Types / Network Device / Device Operation / Control
> Virtqueue / Flow Filter / Flow Filter Match Capabilities Get}.
> >
> > The flow filters are grouped using a flow filter group. Each flow
> > filter group has a priority. The device first applies the flow
> > filters of the highest @@ -1224,7 +1228,136 @@ \subsubsection{Flow
> Filter}\label{sec:Device Types / Network Device / Device Ope
> > the flow filters in group_C, the flow filters of next level group_B are
> applied.
> > \end{itemize}
> >
> > -\label{sec:Device Types / Network Device / Device Operation / Control
> > Virtqueue / Setting Promiscuous Mode}%old label for latexdiff
> > +\paragraph{Match Types and Fields}\label{sec:Device Types / Network
> > +Device / Device Operation / Flow Filter / Match Types and Fields}
> > +
> > +\begin{lstlisting}
> > +struct virtio_net_ff_match_type_cap {
> > + le16 type;
> > + u8 mask_supported;
> > + u8 reserved[5];
> > + le64 fields_bmap;
> > +};
> > +\end{lstlisting}
> > +
> > +The \field{type} corresponds to following table:
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Type & Name & Description \\
> > +\hline \hline
> > +0 & VIRTIO_NET_FF_ETH_HDR & Ethernet header of the packet \\
> > +\hline
> > +0x1 & VIRTIO_NET_FF_VLAN_TAG_HDR & VLAN tag of the packet \\
> > +\hline
> > +0x200 & VIRTIO_NET_FF_IPV4_HDR & IPv4 header of the packet \\
> > +\hline
> > +0x300 & VIRTIO_NET_FF_IPV6_HDR & IPv6 header of the packet \\
> > +\hline
> > +0x400 & VIRTIO_NET_FF_TCP_HDR & TCP header of the packet \\
> > +\hline
> > +0x500 & VIRTIO_NET_FF_UDP_HDR & UDP header of the packet \\
> > +\hline
> > +other & - & reserved \\
> > +\hline
> > +\end{tabular}
> > +
> > +When the \field{mask_supported} is set, for the specific
> > +\field{type}, the device can perform masking packet fields with the
> > +mask supplied in the flow filter match entry.
> > +
> > +For each \field{type} the \field{fields_bmap} indicates supported
> > +fields of the packet header which can be matched.
> > +
> > +For the \field{type} of VIRTIO_NET_FF_ETH_HDR, header fields are
> > +represented by a bitmap in \field{fields_bmap} are following:
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Bit & Name & Description \\
> > +\hline \hline
> > +0 & VIRTIO_NET_FF_DST_MAC & Destination MAC address in the packet \\
> > +\hline
> > +1 & VIRTIO_NET_FF_SRC_MAC & Source MAC address in the packet \\
> > +\hline
> > +2 & VIRTIO_NET_FF_ETHER_TYPE & Ether type in the packet \\
> > +\hline
> > +other & - & reserved \\
> > +\hline
> > +\end{tabular}
> > +
> > +For the \field{type} of VIRTIO_NET_FF_VLAN_TAG_HDR, VLAN tag fields
> > +are represented by a bitmap in \field{fields_bmap} are following:
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Bit & Name & Description \\
> > +\hline \hline
> > +0 & VIRTIO_NET_FF_VLAN_TAG_TCI & Vlan tag TCI 16-bit field \\
> > +\hline
> > +other & - & reserved \\
> > +\hline
> > +\end{tabular}
> > +
> > +For the \field{type} of VIRTIO_NET_FF_IPV4_HDR, header fields are
> > +represented by a bitmap in \field{fields_bmap} are following:
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Bit & Name & Description \\
> > +\hline \hline
> > +0 & VIRTIO_NET_FF_SRC_IPV4 & Source IPV4 address in the packet \\
> > +\hline
> > +1 & VIRTIO_NET_FF_DST_IPV4 & Destination IPV4 address in the packet \\
> > +\hline
> > +other & - & reserved \\
> > +\hline
> > +\end{tabular}
> > +
> > +For the \field{type} of VIRTIO_NET_FF_IPV6_HDR, header fields are
> > +represented by a bitmap in \field{fields_bmap} are following:
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Bit & Name & Description \\
> > +\hline \hline
> > +0 & VIRTIO_NET_FF_SRC_IPV6 & Source IPV6 address in the packet \\
> > +\hline
> > +1 & VIRTIO_NET_FF_DST_IPV6 & Destination IPV6 address in the packet \\
> > +\hline
> > +other & - & reserved \\
> > +\hline
> > +\end{tabular}
> > +
> > +For the \field{type} of VIRTIO_NET_FF_TCP_HDR, header fields are
> > +represented by a bitmap in \field{fields_bmap} are following:
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Bit & Name & Description \\
> > +\hline \hline
> > +0 & VIRTIO_NET_FF_SRC_TCP_PORT & Source TCP port in the packet \\
> > +\hline
> > +1 & VIRTIO_NET_FF_DST_TCP_PORT & Destination TCP port in the packet
> \\
> > +\hline
> > +other & - & reserved \\
> > +\hline
> > +\end{tabular}
> > +
> > +For the \field{type} of VIRTIO_NET_FF_UDP_HDR, header fields are
> > +represented by a bitmap in \field{fields_bmap} are following:
> > +
> > +\begin{tabular}{|l|l|l|}
> > +\hline
> > +Bit & Name & Description \\
> > +\hline \hline
> > +0 & VIRTIO_NET_FF_SRC_UDP_PORT & Source UDP port in the packet \\
> > +\hline
> > +1 & VIRTIO_NET_FF_DST_UDP_PORT & Destination UDP port in the packet
> \\
> > +\hline
> > +other & - & reserved \\
> > +\hline
> > +\end{tabular}
> > +
>
> This is such an elaborate structure to report just 12 read only bits.
> Please let's just follow the example of le32 supported_tunnel_types and add
> l32 supported_flow_control.
>
supported_tunnel_types was let go because cvq is efficient.
None of these fields are needed for init time configuration of the driver before DRIVER_OK.
It is a very narrow view of 12 bits. It is going to grow and many.
This is far more structured for each type done here.
>
> After you were trying to add kilobytes to megabytes of memory - I see little
> reason to save 12 RO bits that can be shared by any number of VFs.
>
Completely wrong reason and very late review and also does not align with every other command we did.
> However, I do think we should create an option to access config space over
> DMA (preferably admin commands). Let's add this quickly and then we don't
> need to worry about legacy guests accessing flow filter through MMIO.
CVQ is already there as single interface forget and set for rss, rss context, vq moderation, statistics, flow filter caps and more.
No reason to bifurcate.
I won't be able to absorb this comment of DMA interface.
If I discuss further, I will repeat the whole document [1] and I will avoid that now.
[1] https://docs.google.com/document/d/1Iyn-l3Nm0yls3pZaul4lZiVj8x1s73Ed6rOsmn6LfXc/edit#heading=h.qexbtyc2jpwr
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]