All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] [PATCH v6] virtio-network: Clarify VLAN filter table configuration
@ 2023-01-08 14:46 Parav Pandit
  2023-01-08 15:38 ` [virtio-comment] " Michael S. Tsirkin
  0 siblings, 1 reply; 3+ messages in thread
From: Parav Pandit @ 2023-01-08 14:46 UTC (permalink / raw)
  To: mst, virtio-dev; +Cc: virtio-comment, si-wei.liu, Parav Pandit

The filtering behavior of the VLAN filter commands is not very clear as
discussed in thread [1].

Hence, add the command description and device requirements for it.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg912392.html
Fixes: https://github.com/oasis-tcs/virtio-spec/issues/147
Suggested-by: Si-Wei Liu <si-wei.liu@oracle.com>
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
changelog:
v5->v6:
- removed unwanted article
v4->v5:
- reworded 'vlan filtering table' to 'vlan filter table' to match
  to the existing description about vlan filtering
- remove confusing text around VLAN_DEL command description
- added missing article
- reword device match configuration to device configuration
- changed 'found' to 'present' and 'not found' to 'absent' to
  consider vlan filter table as config table rather
  than search table
v3->v4:
- added description for accepting vlan tagged packets when vlan
  filter is not negotiated
v2->v3:
- corrected grammar
- simplified description for untagged packets
v1->v2:
- adapt to new file path
v0->v1:
- added missing conformance section link
---
 .../virtio-network/device-conformance.tex     |  1 +
 device-types/virtio-network/device.tex        | 20 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/device-types/virtio-network/device-conformance.tex b/device-types/virtio-network/device-conformance.tex
index c686377..54f6783 100644
--- a/device-types/virtio-network/device-conformance.tex
+++ b/device-types/virtio-network/device-conformance.tex
@@ -9,6 +9,7 @@
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Incoming Packets}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Packet Receive Filtering}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Setting MAC Address Filtering}
+\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
diff --git a/device-types/virtio-network/device.tex b/device-types/virtio-network/device.tex
index e7cdd6f..8a7c6ac 100644
--- a/device-types/virtio-network/device.tex
+++ b/device-types/virtio-network/device.tex
@@ -1210,6 +1210,26 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 Both the VIRTIO_NET_CTRL_VLAN_ADD and VIRTIO_NET_CTRL_VLAN_DEL
 command take a little-endian 16-bit VLAN id as the command-specific-data.
 
+VIRTIO_NET_CTRL_VLAN_ADD command adds the specified VLAN to the
+VLAN filter table.
+
+VIRTIO_NET_CTRL_VLAN_DEL command removes the specified VLAN from
+the VLAN filter table.
+
+\devicenormative{\subparagraph}{VLAN Filtering}{Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering}
+
+When VIRTIO_NET_F_CTRL_VLAN is not negotiated, the device accepts
+all VLAN tagged packets as per the device configuration.
+
+When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
+an empty VLAN filter table.
+
+When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device accepts all
+VLAN tagged packets whose VLAN tag is present in the VLAN
+filter table; the device should drop all VLAN tagged packets
+whose VLAN tag is absent in the VLAN filter table. The
+VLAN filter table applies only to VLAN tagged packets.
+
 \subparagraph{Legacy Interface: VLAN Filtering}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering / Legacy Interface: VLAN Filtering}
 When using the legacy interface, transitional devices and drivers
 MUST format the VLAN id
-- 
2.26.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [virtio-comment] Re: [PATCH v6] virtio-network: Clarify VLAN filter table configuration
  2023-01-08 14:46 [virtio-comment] [PATCH v6] virtio-network: Clarify VLAN filter table configuration Parav Pandit
@ 2023-01-08 15:38 ` Michael S. Tsirkin
  2023-01-08 17:06   ` [virtio-comment] " Parav Pandit
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2023-01-08 15:38 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-dev, virtio-comment, si-wei.liu

On Sun, Jan 08, 2023 at 04:46:59PM +0200, Parav Pandit wrote:
> The filtering behavior of the VLAN filter commands is not very clear as
> discussed in thread [1].
> 
> Hence, add the command description and device requirements for it.
> 
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg912392.html
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/147
> Suggested-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> ---
> changelog:
> v5->v6:
> - removed unwanted article
> v4->v5:
> - reworded 'vlan filtering table' to 'vlan filter table' to match
>   to the existing description about vlan filtering
> - remove confusing text around VLAN_DEL command description
> - added missing article
> - reword device match configuration to device configuration
> - changed 'found' to 'present' and 'not found' to 'absent' to
>   consider vlan filter table as config table rather
>   than search table
> v3->v4:
> - added description for accepting vlan tagged packets when vlan
>   filter is not negotiated
> v2->v3:
> - corrected grammar
> - simplified description for untagged packets
> v1->v2:
> - adapt to new file path
> v0->v1:
> - added missing conformance section link
> ---

Almost there. A minor nit:

>  .../virtio-network/device-conformance.tex     |  1 +
>  device-types/virtio-network/device.tex        | 20 +++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/device-types/virtio-network/device-conformance.tex b/device-types/virtio-network/device-conformance.tex
> index c686377..54f6783 100644
> --- a/device-types/virtio-network/device-conformance.tex
> +++ b/device-types/virtio-network/device-conformance.tex
> @@ -9,6 +9,7 @@
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Incoming Packets}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Packet Receive Filtering}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Setting MAC Address Filtering}
> +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> diff --git a/device-types/virtio-network/device.tex b/device-types/virtio-network/device.tex
> index e7cdd6f..8a7c6ac 100644
> --- a/device-types/virtio-network/device.tex
> +++ b/device-types/virtio-network/device.tex
> @@ -1210,6 +1210,26 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  Both the VIRTIO_NET_CTRL_VLAN_ADD and VIRTIO_NET_CTRL_VLAN_DEL
>  command take a little-endian 16-bit VLAN id as the command-specific-data.
>  
> +VIRTIO_NET_CTRL_VLAN_ADD command adds the specified VLAN to the
> +VLAN filter table.
> +
> +VIRTIO_NET_CTRL_VLAN_DEL command removes the specified VLAN from
> +the VLAN filter table.
> +
> +\devicenormative{\subparagraph}{VLAN Filtering}{Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering}
> +
> +When VIRTIO_NET_F_CTRL_VLAN is not negotiated, the device accepts
> +all VLAN tagged packets as per the device configuration.
> +
> +When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
> +an empty VLAN filter table.
> +
> +When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device accepts all
> +VLAN tagged packets whose VLAN tag is present in the VLAN
> +filter table; the device should drop all VLAN tagged packets
> +whose VLAN tag is absent in the VLAN filter table. The
> +VLAN filter table applies only to VLAN tagged packets.
> +

We need to make them normative statements. As written they are not.
Either MUST or SHOULD in this case. I am guessing MUST since this
was always the intent we just never spelled it out?

>  \subparagraph{Legacy Interface: VLAN Filtering}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering / Legacy Interface: VLAN Filtering}
>  When using the legacy interface, transitional devices and drivers
>  MUST format the VLAN id
> -- 
> 2.26.2


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [virtio-comment] RE: [PATCH v6] virtio-network: Clarify VLAN filter table configuration
  2023-01-08 15:38 ` [virtio-comment] " Michael S. Tsirkin
@ 2023-01-08 17:06   ` Parav Pandit
  0 siblings, 0 replies; 3+ messages in thread
From: Parav Pandit @ 2023-01-08 17:06 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-dev, virtio-comment, si-wei.liu



> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Sunday, January 8, 2023 10:39 AM
> 
> > +VIRTIO_NET_CTRL_VLAN_ADD command adds the specified VLAN to the
> VLAN
> > +filter table.
> > +
> > +VIRTIO_NET_CTRL_VLAN_DEL command removes the specified VLAN from
> the
> > +VLAN filter table.
> > +
Above two are functional description not placed in the requirements. So no need for normative.

> > +\devicenormative{\subparagraph}{VLAN Filtering}{Device Types /
> > +Network Device / Device Operation / Control Virtqueue / VLAN
> > +Filtering}
> > +
> > +When VIRTIO_NET_F_CTRL_VLAN is not negotiated, the device accepts all
> > +VLAN tagged packets as per the device configuration.
> > +
Above one adding must looks fine.

> > +When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with an
> > +empty VLAN filter table.
> > +
I am inclined to move this out of requirements and write in the description along with add/delete command.

> > +When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device accepts all
> > +VLAN tagged packets whose VLAN tag is present in the VLAN filter
> > +table; 

I will add must for above line.

> > the device should drop all VLAN tagged packets whose VLAN tag
> > +is absent in the VLAN filter table. The VLAN filter table applies
> > +only to VLAN tagged packets.
> > +
> 
Above one is written as should will change to "SHOULD".

> We need to make them normative statements. As written they are not.
> Either MUST or SHOULD in this case. I am guessing MUST since this was always
> the intent we just never spelled it out?

Right.

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-01-08 17:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-08 14:46 [virtio-comment] [PATCH v6] virtio-network: Clarify VLAN filter table configuration Parav Pandit
2023-01-08 15:38 ` [virtio-comment] " Michael S. Tsirkin
2023-01-08 17:06   ` [virtio-comment] " Parav Pandit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.