virtio-network: Clarify VLAN filter table configuration

PATCH Superseded Parav Pandit 2 participants 5 messages Jan 10, 2023 – Jan 23, 2023

lore

Share & tools

lore

Parav Pandit

author

2023-01-10 23:09 UTC #0 lore raw

PATCH v9 1/1 virtio-network: Clarify VLAN filter table configuration

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://lists.oasis-open.org/archives/virtio-dev/202301/msg00136.html

Fixes https://github.com/oasis-tcs/virtio-spec/issues/147

Reviewed-by Si-Wei Liu <[email protected]>

Acked-by Michael S. Tsirkin <[email protected]>

Signed-off-by Parav Pandit <[email protected]>

---
changelog:
v8->v9:
- rebased on top of [1]
- updated commit message to newer link
v7->v8:
- Fixed grammar
v6->v7:
- Moved VLAN filter table description from requirements to device descrption section
- Added MUST and SHOULD to device requirements
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

---
 device-types/virtio-network/description.tex   | 22 ++++++++++++++++++-
 .../virtio-network/device-conformance.tex     |  1 +
 2 files changed, 22 insertions(+), 1 deletion(-)

device-types/virtio-network/description.tex +21 -1

index 7409f2a..970090c 100644
@@ -1194,7 +1194,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \paragraph{VLAN Filtering}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering}
 
 If the driver negotiates the VIRTIO_NET_F_CTRL_VLAN feature, it
-can control a VLAN filter table in the device.
+can control a VLAN filter table in the device. The VLAN filter
+table applies only to VLAN tagged packets.
+
+When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
+an empty VLAN filter table.
 
 \begin{note}
 Similar to the MAC address based filtering, the VLAN filtering

@@ -1210,6 +1214,22 @@ \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 MUST
+accept all VLAN tagged packets as per the device configuration.
+
+When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device MUST
+accept all VLAN tagged packets whose VLAN tag is present in
+the VLAN filter table and SHOULD drop all VLAN tagged packets
+whose VLAN tag is absent in the VLAN filter table.
+
 \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

device-types/virtio-network/device-conformance.tex +1 -0

index c686377..54f6783 100644
@@ -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}

Signature · 6 lines

2.26.2


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Halil Pasic

2023-01-23 11:40 UTC #1 lore raw

Quoted 7 lines · On Wed, 11 Jan 2023 01:09:45 +0200 Parav Pandit <[email protected]> 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://lists.oasis-open.org/archives/virtio-dev/202301/msg00136.html

Sorry I could not find the discussion. Can you point me to the email where the discussion starts?

Quoted 64 lines

Fixes https://github.com/oasis-tcs/virtio-spec/issues/147
Suggested-by Si-Wei Liu <[email protected]>
Reviewed-by Si-Wei Liu <[email protected]>
Acked-by Michael S. Tsirkin <[email protected]>
Signed-off-by Parav Pandit <[email protected]>
---
changelog:
v8->v9:
- rebased on top of [1]
- updated commit message to newer link
v7->v8:
- Fixed grammar
v6->v7:
- Moved VLAN filter table description from requirements to device descrption section
- Added MUST and SHOULD to device requirements
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
---
 device-types/virtio-network/description.tex   | 22 ++++++++++++++++++-
 .../virtio-network/device-conformance.tex     |  1 +
 2 files changed, 22 insertions(+), 1 deletion(-)
device-types/virtio-network/description.tex +5 -1
index 7409f2a..970090c 100644
@@ -1194,7 +1194,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \paragraph{VLAN Filtering}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / VLAN Filtering}
 
 If the driver negotiates the VIRTIO_NET_F_CTRL_VLAN feature, it
-can control a VLAN filter table in the device.
+can control a VLAN filter table in the device. The VLAN filter
+table applies only to VLAN tagged packets.
+
+When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device starts with
+an empty VLAN filter table.
 
 \begin{note}
 Similar to the MAC address based filtering, the VLAN filtering
@@ -1210,6 +1214,22 @@ \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.
 

device-types/virtio-network/description.tex \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi · line 1226

Quoted 10 lines

device-types/virtio-network/description.tex +10 -0

+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 MUST
+accept all VLAN tagged packets as per the device configuration.

I find "as per the device configuration" difficult to comprehend. Maybe we can work on this with an editorial. What are you trying to express here? On one hand you say "the device MUST accept all VLAN tagged packets" on the other hand "per the device configuration" may explain, or may relativize and restrict that statement.

Would
"When VIRTIO_NET_F_CTRL_VLAN is not negotiated, VLAN filtering is not applied. I.e. VLAN tags are ignored by the device." also work?

device-types/virtio-network/description.tex \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi · line 1232

Quoted 6 lines

device-types/virtio-network/description.tex +6 -0

+
+When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device MUST
+accept all VLAN tagged packets whose VLAN tag is present in
+the VLAN filter table and SHOULD drop all VLAN tagged packets
+whose VLAN tag is absent in the VLAN filter table.
+

Maybe it would be more fortunate to make these statements with respect to the VLAN filtering or VLAN filter, and not with respect to the device.

For example, if the package is otherwise broken or should be dropped for different reasons (maybe MAC filtering?).

BTW why SHOULD drop?

Quoted 8 lines

device-types/virtio-network/description.tex +0 -0

 \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

device-types/virtio-network/device-conformance.tex +0 -0

index c686377..54f6783 100644

device-types/virtio-network/device-conformance.tex line 15

Quoted 7 lines

device-types/virtio-network/device-conformance.tex +1 -0

@@ -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}

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: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
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/

Parav Pandit

author

2023-01-23 12:41 UTC #2 lore raw

Quoted 10 lines

From: Halil Pasic <[email protected]>
Sent: Monday, January 23, 2023 6:40 AM
Quoted 4 lines · On Wed, 11 Jan 2023 01:09:45 +0200 Parav Pandit <[email protected]> wrote:
The filtering behavior of the VLAN filter commands is not very clear as discussed in thread [1].

Quoted 8 lines

Quoted 4 lines
Hence, add the command description and device requirements for it.
[1]
https://lists.oasis-open.org/archives/virtio-dev/202301/msg00136.html
Sorry I could not find the discussion. Can you point me to the email where the discussion starts?

Latest patch is for this change is : https://lists.oasis-open.org/archives/virtio-dev/202301/msg00231.html

There was past discussion at https://www.mail-archive.com/[email protected]/msg913433.html

Quoted 4 lines

Quoted 4 lines
+\devicenormative{\subparagraph}{VLAN Filtering}{Device Types /
(quoted diff) +3 -0
+Network Device / Device Operation / Control Virtqueue / VLAN
+Filtering}
+

Quoted 9 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+When VIRTIO_NET_F_CTRL_VLAN is not negotiated, the device MUST accept
+all VLAN tagged packets as per the device configuration.
I find "as per the device configuration" difficult to comprehend. Maybe we can work on this with an editorial. What are you trying to express here? On one hand you say "the device MUST accept all VLAN tagged packets" on the other hand "per the device configuration" may explain, or may relativize and restrict that statement.

From VLAN filtering perspective, it should accept all packets, but this is subject to other device configuration such as MAC programming. The current one is fine that cover both the aspects to VLAN specific and other config.

Quoted 5 lines

Would
"When VIRTIO_NET_F_CTRL_VLAN is not negotiated, VLAN filtering is not applied. I.e. VLAN tags are ignored by the device."
also work?

No. its not about ignoring "VLAN tags". Its about dealing with "packets" that starts with the VLAN tag.

Quoted 10 lines

Review
Quoted 6 lines
(quoted diff) +6 -0
+
+When VIRTIO_NET_F_CTRL_VLAN is negotiated, the device MUST accept all
+VLAN tagged packets whose VLAN tag is present in the VLAN filter
+table and SHOULD drop all VLAN tagged packets whose VLAN tag is
+absent in the VLAN filter table.
+
Maybe it would be more fortunate to make these statements with respect to the VLAN filtering or VLAN filter, and not with respect to the device.

Quoted 3 lines

For example, if the package is otherwise broken or should be dropped for different reasons (maybe MAC filtering?).

Here the context is only VLAN filter. Packet may be dropped for other reasons.
We preferably want to stay away from describing it such details here, because steering features are expected to grow. And talking about each of them in cross section just complicates it.

In near future, I plan to further improve and add a steering section, that will describe the hierarchy or interworking of these, unrelated to this.

Quoted 1 line

BTW why SHOULD drop?

This is the offload feature to drop such packets so that OS driver doesn't need to do the filtering work. :)

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.

Halil Pasic

2023-01-23 14:49 UTC #3 lore raw

Quoted 12 lines · On Mon, 23 Jan 2023 12:41:16 +0000 Parav Pandit <[email protected]> wrote:

Quoted 12 lines
From: Halil Pasic <[email protected]>
Sent: Monday, January 23, 2023 6:40 AM
Quoted 6 lines · On Wed, 11 Jan 2023 01:09:45 +0200 Parav Pandit <[email protected]> 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.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
[1]
https://lists.oasis-open.org/archives/virtio-dev/202301/msg00136.html
Sorry I could not find the discussion. Can you point me to the email where the discussion starts?
Latest patch is for this change is : https://lists.oasis-open.org/archives/virtio-dev/202301/msg00231.html

Sorry, my bad. Should I repost my feedback on v10 or should we continue here? AFAICT v10 is a rebase on the directory structure stuff, so the text should be the same.

Quoted 1 line

There was past discussion at https://www.mail-archive.com/[email protected]/msg913433.html

That is a completely different discussion than provided under [1] in the commit message. Right? So the link provided is just wrong?

Regards,
Halil

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.

Parav Pandit

author

2023-01-23 14:54 UTC #4 lore raw

Quoted 23 lines

From: Halil Pasic <[email protected]>
Sent: Monday, January 23, 2023 9:50 AM
Quoted 16 lines · On Mon, 23 Jan 2023 12:41:16 +0000 Parav Pandit <[email protected]> wrote:
Quoted 16 lines
From: Halil Pasic <[email protected]>
Sent: Monday, January 23, 2023 6:40 AM
Quoted 9 lines · On Wed, 11 Jan 2023 01:09:45 +0200 Parav Pandit <[email protected]> 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://lists.oasis-open.org/archives/virtio-dev/202301/msg00136.h
tml

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Sorry I could not find the discussion. Can you point me to the email where the discussion starts?
Latest patch is for this change is :
https://lists.oasis-open.org/archives/virtio-dev/202301/msg00231.html
Sorry, my bad. Should I repost my feedback on v10 or should we continue here?

Feedback on v10 is better as it's the latest.

Quoted 2 lines

AFAICT v10 is a rebase on the directory structure stuff, so the text should be the same.

Right. Text is same.

Quoted 6 lines

Quoted 3 lines
There was past discussion at
https://www.mail-archive.com/[email protected]/msg913433.html
That is a completely different discussion than provided under [1] in the commit message. Right? So the link provided is just wrong?

To keep things short, I think v10 patch and github issue description is good enough details.

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.