[virtio-dev] [PATCH 3/3] transport-mmio: Refer to the vq by its number

[Parav Pandit]

Currently specification uses virtqueue index and
number interchangeably to refer to the virtqueue.

Instead refer to it by its number.

This patch is on top of [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00527.html

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit 
---
 transport-mmio.tex | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index c59975e..324cecf 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -96,7 +96,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 bits accessible by writing to \field{DriverFeatures}.
   }
   \hline
-  \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
+  \mmioreg{QueueSel}{Virtual queue number}{0x030}{W}{%
 Writing to this register selects the virtual queue that the
 following operations on \field{QueueNumMax}, \field{QueueNum}, 
\field{QueueReady},
 \field{QueueDescLow}, \field{QueueDescHigh}, \field{QueueDriverlLow}, 
\field{QueueDriverHigh},
@@ -130,7 +130,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 there are new buffers to process in a queue.
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
-the value written is the queue index.
+the value written is the queue number.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
 the \field{Notification data} value has the following format:
@@ -373,7 +373,7 @@ \subsubsection{Available Buffer 
Notifications}\label{sec:Virtio Transport Option
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
 the driver sends an available buffer notification to the device by writing
-the 16-bit virtqueue index
+the 16-bit virtqueue number
 of the queue to be notified to \field{QueueNotify}.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
@@ -451,7 +451,7 @@ \subsection{Legacy interface}\label{sec:Virtio Transport 
Options / Virtio Over M
 (see QueuePFN).
   }
   \hline
-  \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
+  \mmioreg{QueueSel}{Virtual queue number}{0x030}{W}{%
 Writing to this register selects the virtual queue that the
 following operations on the \field{QueueNumMax}, \field{QueueNum}, 
\field{QueueAlign}
 and \field{QueuePFN} registers apply to. The index
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH 2/3] transport-mmio: Rename QueueNum register

[Parav Pandit]

Currently, the specification uses virtqueue index and number
interchangeably to refer to the virtqueue.

It is better to identify it using one terminology.

Two registers QueueNumMax and QueueNum actually reflect the queue size
or queue depth indicating max and actual number of entries in the queue.
Equivalent register in PCI transport is named differently as queue_size.

To bring consistency between pci and mmio transport, and to avoid
confusion between number and index, rename the QueueNumMax and QueueNum
registers to QueueSizeMax and QueueSize respectively.

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00527.html

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit 
---
 transport-mmio.tex | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index 65bae54..c59975e 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -104,14 +104,14 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 number of the first queue is zero (0x0).
   }
   \hline
-  \mmioreg{QueueNumMax}{Maximum virtual queue size}{0x034}{R}{%
+  \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
 Reading from the register returns the maximum size (number of
 elements) of the queue the device is ready to process or
 zero (0x0) if the queue is not available. This applies to the
 queue selected by writing to \field{QueueSel}.
   }
   \hline
-  \mmioreg{QueueNum}{Virtual queue size}{0x038}{W}{%
+  \mmioreg{QueueSize}{Virtual queue size}{0x038}{W}{%
 Queue size is the number of elements in the queue.
 Writing to this register notifies the device what size of the
 queue the driver will use. This applies to the queue selected by
@@ -459,7 +459,7 @@ \subsection{Legacy interface}\label{sec:Virtio Transport 
Options / Virtio Over M
 .
   }
   \hline
-  \mmioreg{QueueNumMax}{Maximum virtual queue size}{0x034}{R}{%
+  \mmioreg{QueueSizeMax}{Maximum virtual queue size}{0x034}{R}{%
 Reading from the register returns the maximum size of the queue
 the device is ready to process or zero (0x0) if the queue is not
 available. This applies to the queue selected by writing to
@@ -467,7 +467,7 @@ \subsection{Legacy interface}\label{sec:Virtio Transport 
Options / Virtio Over M
 (0x0), so when the queue is not actively used.
   }
   \hline
-  \mmioreg{QueueNum}{Virtual queue size}{0x038}{W}{%
+  \mmioreg{QueueSize}{Virtual queue size}{0x038}{W}{%
 Queue size is the number of elements in the queue.
 Writing to this register notifies the device what size of the
 queue the driver will use. This applies to the queue selected by
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH 0/3] Rename queue index to queue number

[Parav Pandit]

1. Currently, virtqueue is identified between driver and device
interchangeably using either number of index terminology.

2. Between PCI and MMIO transport the queue size (depth) is
defined as queue_size and QueueNum respectively.

To avoid confusion and to have consistency, unify them to use as Number.

Solution:
Use virtqueue number description, and rename MMIO register as QueueSize.

Patch summary:
patch-1 renames index to number for pci transport
patch-2 renames mmio register from Num to Size
patch-3 renames index to number for mmio transport

Please review.
This series fixes the issue [1].

This series is on top of [2].

[1] https://github.com/oasis-tcs/virtio-spec/issues/163
[2] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00527.html

---
Cornelia:
I was not sure about ccw for vq_config_block and vq_info_block structures
index field refers to the queue number or not.
Can you please clarify?

If it vqn, I will send v1 by replacing index to vqn to be
consistent with other part of the spec which also uses vqn.

Parav Pandit (3):
  transport-pci: Refer to the vq by its number
  transport-mmio: Rename QueueNum register
  transport-mmio: Refer to the vq by its number

 transport-mmio.tex | 16 
 transport-pci.tex  |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH 1/3] transport-pci: Refer to the vq by its number

[Parav Pandit]

Currently specification uses virtqueue index and
number interchangeably to refer to the virtqueue.

Instead refer to it by its number.

This patch is on top of [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00527.html

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/163
Signed-off-by: Parav Pandit 
---
 transport-pci.tex | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index da1486a..c9e112a 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -1005,7 +1005,7 @@ \subsubsection{Device Initialization}\label{sec:Virtio 
Transport Options / Virti
 The driver typically does this as follows, for each virtqueue a device has:
 
 \begin{enumerate}
-\item Write the virtqueue index (first queue is 0) to \field{queue_select}.
+\item Write the virtqueue number (first queue is 0) to \field{queue_select}.
 
 \item Read the virtqueue size from \field{queue_size}. This controls how big 
the virtqueue is
   (see \ref{sec:Basic Facilities of a Virtio Device / 
Virtqueues}~\nameref{sec:Basic Facilities of a Virtio Device / Virtqueues}). If 
this field is 0, the virtqueue does not exist.
@@ -1035,7 +1035,7 @@ \subsubsection{Available Buffer 
Notifications}\label{sec:Virtio Transport Option
 
 When VIRTIO_F_NOTIFICATION_DATA has not been negotiated,
 the driver sends an available buffer notification to the device by writing
-the 16-bit virtqueue index
+the 16-bit virtqueue number
 of this virtqueue to the Queue Notify address.
 
 When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
@@ -1053,7 +1053,7 @@ \subsubsection{Available Buffer 
Notifications}\label{sec:Virtio Transport Option
 If VIRTIO_F_NOTIF_CONFIG_DATA has been negotiated:
 \begin{itemize}
 \item If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the driver MUST 
use the
-\field{queue_notify_data} value instead of the virtqueue index.
+\field{queue_notify_data} value instead of the virtqueue number.
 \item If VIRTIO_F_NOTIFICATION_DATA has been negotiated, the driver MUST set 
the
 \field{vqn} field to the \field{queue_notify_data} value.
 \end{itemize}
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

RE: [virtio-dev] [PATCH v2] virtio-net: define the VIRTIO_NET_F_CTRL_RX_EXTRA feature bit

[Parav Pandit]

> From: virtio-dev@lists.oasis-open.org  On
> Behalf Of Alvaro Karsz
> 
> The VIRTIO_NET_F_CTRL_RX_EXTRA feature bit is mentioned in the spec since
> version 1.0, but it's not properly defined.
> 
> This patch defines the feature bit and defines the dependency on
> VIRTIO_NET_F_CTRL_VQ.
> 
> Since this dependency is missing in previous versions, we add it now as a
> "SHOULD".
> 
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/162
> 
> Signed-off-by: Alvaro Karsz 
> ---
> v2:
>   - Rephrase commit log, no changes to patch body.
> 
>  device-types/net/description.tex | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/device-types/net/description.tex 
> b/device-types/net/description.tex
> index 8487ccd..cf37da5 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -74,6 +74,8 @@ \subsection{Feature bits}\label{sec:Device Types /
> Network Device / Feature bits
> 
>  \item[VIRTIO_NET_F_CTRL_VLAN (19)] Control channel VLAN filtering.
> 
> +\item[VIRTIO_NET_F_CTRL_RX_EXTRA (20)]   Control channel RX extra
> mode support.
> +
>  \item[VIRTIO_NET_F_GUEST_ANNOUNCE(21)] Driver can send gratuitous
>  packets.
> 
> @@ -259,6 +261,9 @@ \subsection{Device configuration
> layout}\label{sec:Device Types / Network Device  The device SHOULD NOT
> offer VIRTIO_NET_F_HASH_REPORT if it  does not offer
> VIRTIO_NET_F_CTRL_VQ.
> 
> +The device SHOULD NOT offer VIRTIO_NET_F_CTRL_RX_EXTRA if it does not
> +offer VIRTIO_NET_F_CTRL_VQ.
> +
>  \drivernormative{\subsubsection}{Device configuration layout}{Device Types /
> Network Device / Device configuration layout}
> 
>  A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
> @@ -295,6 +300,9 @@ \subsection{Device configuration
> layout}\label{sec:Device Types / Network Device  A driver SHOULD NOT
> negotiate VIRTIO_NET_F_HASH_REPORT if it  does not negotiate
> VIRTIO_NET_F_CTRL_VQ.
> 
> +A driver SHOULD NOT negotiate VIRTIO_NET_F_CTRL_RX_EXTRA if it does not
> +negotiate VIRTIO_NET_F_CTRL_VQ.
> +
>  \subsubsection{Legacy Interface: Device configuration 
> layout}\label{sec:Device
> Types / Network Device / Device configuration layout / Legacy Interface: 
> Device
> configuration layout}  \label{sec:Device Types / Block Device / Feature bits /
> Device configuration layout / Legacy Interface: Device configuration layout}
> When using the legacy interface, transitional devices and drivers
> --
> 2.34.1

Reviewed-by: Parav Pandit 


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] Re: [PATCH v9] virtio-net: support inner header hash

[Heng Qi]

在 2023/2/23 上午10:50, Jason Wang 写道:

Hi:

在 2023/2/22 14:46, Heng Qi 写道:

Hi, Jason. Long time no see. :)

在 2023/2/22 上午11:22, Jason Wang 写道:


在 2023/2/22 01:50, Michael S. Tsirkin 写道:

On Sat, Feb 18, 2023 at 10:37:15PM +0800, Heng Qi wrote:

+\subparagraph{Security risks between encapsulated packets and RSS}
+There may be potential security risks when encapsulated packets 
using RSS to
+select queues for placement. When a user inside a tunnel tries to 
control the



What do you mean by "user" here? Is it a remote or local one?



I mean a remote attacker who is not under the control of the tunnel 
owner.



Anything may the tunnel different? I think this can happen even 
without tunnel (and even with single queue).


I agree.



How to mitigate those attackers seems more like a implementation 
details where might require fair queuing or other QOS technology which 
has been well studied.


I am also not sure whether this point needs to be focused on in the 
spec, and I see that the protection against tunnel DoS is more protected 
outside the device,

but it seems to be okay to give some attack reminders.

Thanks.



It seems out of the scope of the spec (unless we want to let driver 
manageable QOS).


Thanks




Thanks.



+enqueuing of encapsulated packets, then the user can flood the 
device with invaild
+packets, and the flooded packets may be hashed into the same 
queue as packets in

+other normal tunnels, which causing the queue to overflow.
+
+This can pose several security risks:
+\begin{itemize}
+\item  Encapsulated packets in the normal tunnels cannot be 
enqueued due to queue

+   overflow, resulting in a large amount of packet loss.
+\item  The delay and retransmission of packets in the normal 
tunnels are extremely increased.
+\item  The user can observe the traffic information and enqueue 
information of other normal

+   tunnels, and conduct targeted DoS attacks.
+\end{\itemize}
+

Hmm with this all written out it sounds pretty severe.



I think we need first understand whether or not it's a problem that 
we need to solve at spec level:


1) anything make encapsulated packets different or why we can't hit 
this problem without encapsulation


2) whether or not it's the implementation details that the spec 
doesn't need to care (or how it is solved in real NIC)


Thanks



At this point with no ways to mitigate, I don't feel this is something
e.g. Linux can enable.  I am not going to nack the spec patch if
others  find this somehow useful e.g. for dpdk.
How about CC e.g. dpdk devs or whoever else is going to use this
and asking them for the opinion?







-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v4 4/6] transport-pci: Fix spellings and white spaces

[Parav Pandit]

Now that we have individual files, fix reported spelling errors.

While at it, remove trailing white spaces.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/157
Signed-off-by: Parav Pandit 
---
changelog:
v0->v1:
- removed many trailing white spaces
---
 transport-pci.tex | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/transport-pci.tex b/transport-pci.tex
index 49c35bd..da1486a 100644
--- a/transport-pci.tex
+++ b/transport-pci.tex
@@ -5,7 +5,7 @@ \section{Virtio Over PCI Bus}\label{sec:Virtio Transport 
Options / Virtio Over P
 A Virtio device can be implemented as any kind of PCI device:
 a Conventional PCI device or a PCI Express
 device.  To assure designs meet the latest level
-requirements, see 
+requirements, see
 the PCI-SIG home page at \url{http://www.pcisig.com} for any
 approved changes.
 
@@ -14,7 +14,7 @@ \section{Virtio Over PCI Bus}\label{sec:Virtio Transport 
Options / Virtio Over P
 guest an interface that meets the specification requirements of
 the appropriate PCI specification: \hyperref[intro:PCI]{[PCI]}
 and \hyperref[intro:PCIe]{[PCIe]}
-respectively. 
+respectively.
 
 \subsection{PCI Device Discovery}\label{sec:Virtio Transport Options / Virtio 
Over PCI Bus / PCI Device Discovery}
 
@@ -586,7 +586,7 @@ \subsubsection{ISR status capability}\label{sec:Virtio 
Transport Options / Virti
 
 \devicenormative{\paragraph}{ISR status capability}{Virtio Transport Options / 
Virtio Over PCI Bus / PCI Device Layout / ISR status capability}
 
-The device MUST present at least one VIRTIO_PCI_CAP_ISR_CFG capability.  
+The device MUST present at least one VIRTIO_PCI_CAP_ISR_CFG capability.
 
 The device MUST set the Device Configuration Interrupt bit
 in \field{ISR status} before sending a device configuration
@@ -945,7 +945,7 @@ \subsubsection{Device Initialization}\label{sec:Virtio 
Transport Options / Virti
 \end{lstlisting}
 
 Note that mapping an event to vector might require device to
-allocate internal device resources, and thus could fail. 
+allocate internal device resources, and thus could fail.
 
 \devicenormative{\subparagraph}{MSI-X Vector Configuration}{Virtio Transport 
Options / Virtio Over PCI Bus / PCI-specific Initialization And Device 
Operation / Device Initialization / MSI-X Vector Configuration}
 
@@ -973,7 +973,7 @@ \subsubsection{Device Initialization}\label{sec:Virtio 
Transport Options / Virti
 unless it is impossible for the device to satisfy the mapping
 request.  Devices MUST report mapping
 failures by returning the NO_VECTOR value when the relevant
-\field{config_msix_vector}/\field{queue_msix_vector} field is read. 
+\field{config_msix_vector}/\field{queue_msix_vector} field is read.
 
 \drivernormative{\subparagraph}{MSI-X Vector Configuration}{Virtio Transport 
Options / Virtio Over PCI Bus / PCI-specific Initialization And Device 
Operation / Device Initialization / MSI-X Vector Configuration}
 
@@ -981,7 +981,7 @@ \subsubsection{Device Initialization}\label{sec:Virtio 
Transport Options / Virti
 Driver MAY fall back on using INT\#x interrupts for a device
 which only supports one MSI-X vector (MSI-X Table Size = 0).
 
-Driver MAY intepret the Table Size as a hint from the device
+Driver MAY interpret the Table Size as a hint from the device
 for the suggested number of MSI-X vectors to use.
 
 Driver MUST NOT attempt to map an event to a vector
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v4 6/6] transport-ccw: Fix spellings and white spaces

[Parav Pandit]

Now that we have individual files, fix reported spelling errors.

While at it, remove extra white spaces.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/157
Signed-off-by: Parav Pandit 
---
changelog:
v1->v2:
- remove trailing white spaces
---
 transport-ccw.tex | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/transport-ccw.tex b/transport-ccw.tex
index 93401a4..c492cb9 100644
--- a/transport-ccw.tex
+++ b/transport-ccw.tex
@@ -56,13 +56,13 @@ \subsection{Basic Concepts}\label{sec:Virtio Transport 
Options / Virtio over cha
 \end{tabular}
 
 A virtio-ccw proxy device facilitates:
-\begin{itemize} 
+\begin{itemize}
 \item Discovery and attachment of virtio devices (as described above).
 \item Initialization of virtqueues and transport-specific facilities (using
   virtio-specific channel commands).
 \item Notifications (via hypercall and a combination of I/O interrupts
   and indicator bits).
-\end{itemize} 
+\end{itemize}
 
 \subsubsection{Channel Commands for Virtio}\label{sec:Virtio Transport Options 
/ Virtio
 over channel I/O / Basic Concepts/ Channel Commands for Virtio}
@@ -107,7 +107,7 @@ \subsubsection{Notifications}\label{sec:Virtio Transport 
Options / Virtio
 Host->Guest Notification / Notification via Classic I/O Interrupts} and
 \ref{sec:Virtio Transport Options / Virtio over channel I/O / Device
 Operation / Host->Guest Notification / Notification via Adapter I/O
-Interrupts} respectively. 
+Interrupts} respectively.
 
 Configuration change notifications are done using so-called classic I/O
 interrupts. The initialization is described in section \ref{sec:Virtio
@@ -413,7 +413,7 @@ \subsubsection{Setting Up Indicators}\label{sec:Virtio 
Transport Options / Virti
 \begin{itemize}
 \item a summary indicator byte covering the virtqueues for one or more
   virtio-ccw proxy devices
-\item a set of contigous indicator bits for the virtqueues for a
+\item a set of contiguous indicator bits for the virtqueues for a
   virtio-ccw proxy device
 \end{itemize}
 
@@ -563,7 +563,7 @@ \subsubsection{Guest->Host Notification}\label{sec:Virtio 
Transport Options / Vi
 
 \drivernormative{\paragraph}{Guest->Host Notification}{Virtio Transport 
Options / Virtio over channel I/O / Device Operation / Guest->Host Notification}
 
-For each notification, the driver SHOULD use GPR4 to pass the host cookie 
received in GPR2 from the previous notication.
+For each notification, the driver SHOULD use GPR4 to pass the host cookie 
received in GPR2 from the previous notification.
 
 \begin{note}
 For example:
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v4 1/6] transport-pci: Split PCI transport to its own file

[Parav Pandit]

Place PCI transport specification in its own file to better maintain it.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/157
Signed-off-by: Parav Pandit 
---
 content.tex   | 1161 +
 transport-pci.tex | 1160 
 2 files changed, 1161 insertions(+), 1160 deletions(-)
 create mode 100644 transport-pci.tex

diff --git a/content.tex b/content.tex
index 0c7cdf8..be911e6 100644
--- a/content.tex
+++ b/content.tex
@@ -579,1166 +579,7 @@ \chapter{Virtio Transport Options}\label{sec:Virtio 
Transport Options}
 Virtio can use various different buses, thus the standard is split
 into virtio general and bus-specific sections.
 
-\section{Virtio Over PCI Bus}\label{sec:Virtio Transport Options / Virtio Over 
PCI Bus}
-
-Virtio devices are commonly implemented as PCI devices.
-
-A Virtio device can be implemented as any kind of PCI device:
-a Conventional PCI device or a PCI Express
-device.  To assure designs meet the latest level
-requirements, see 
-the PCI-SIG home page at \url{http://www.pcisig.com} for any
-approved changes.
-
-\devicenormative{\subsection}{Virtio Over PCI Bus}{Virtio Transport Options / 
Virtio Over PCI Bus}
-A Virtio device using Virtio Over PCI Bus MUST expose to
-guest an interface that meets the specification requirements of
-the appropriate PCI specification: \hyperref[intro:PCI]{[PCI]}
-and \hyperref[intro:PCIe]{[PCIe]}
-respectively. 
-
-\subsection{PCI Device Discovery}\label{sec:Virtio Transport Options / Virtio 
Over PCI Bus / PCI Device Discovery}
-
-Any PCI device with PCI Vendor ID 0x1AF4, and PCI Device ID 0x1000 through
-0x107F inclusive is a virtio device. The actual value within this range
-indicates which virtio device is supported by the device.
-The PCI Device ID is calculated by adding 0x1040 to the Virtio Device ID,
-as indicated in section \ref{sec:Device Types}.
-Additionally, devices MAY utilize a Transitional PCI Device ID range,
-0x1000 to 0x103F depending on the device type.
-
-\devicenormative{\subsubsection}{PCI Device Discovery}{Virtio Transport 
Options / Virtio Over PCI Bus / PCI Device Discovery}
-
-Devices MUST have the PCI Vendor ID 0x1AF4.
-Devices MUST either have the PCI Device ID calculated by adding 0x1040
-to the Virtio Device ID, as indicated in section \ref{sec:Device
-Types} or have the Transitional PCI Device ID depending on the device type,
-as follows:
-
-\begin{tabular}{|l|c|}
-\hline
-Transitional PCI Device ID  &  Virtio Device\\
-\hline \hline
-0x1000  &   network device \\
-\hline
-0x1001 &   block device \\
-\hline
-0x1002 & memory ballooning (traditional)  \\
-\hline
-0x1003 &  console   \\
-\hline
-0x1004 & SCSI host  \\
-\hline
-0x1005 &  entropy source\\
-\hline
-0x1009 &   9P transport \\
-\hline
-\end{tabular}
-
-For example, the network device with the Virtio Device ID 1
-has the PCI Device ID 0x1041 or the Transitional PCI Device ID 0x1000.
-
-The PCI Subsystem Vendor ID and the PCI Subsystem Device ID MAY reflect
-the PCI Vendor and Device ID of the environment (for informational purposes by 
the driver).
-
-Non-transitional devices SHOULD have a PCI Device ID in the range
-0x1040 to 0x107f.
-Non-transitional devices SHOULD have a PCI Revision ID of 1 or higher.
-Non-transitional devices SHOULD have a PCI Subsystem Device ID of 0x40 or 
higher.
-
-This is to reduce the chance of a legacy driver attempting
-to drive the device.
-
-\drivernormative{\subsubsection}{PCI Device Discovery}{Virtio Transport 
Options / Virtio Over PCI Bus / PCI Device Discovery}
-Drivers MUST match devices with the PCI Vendor ID 0x1AF4 and
-the PCI Device ID in the range 0x1040 to 0x107f,
-calculated by adding 0x1040 to the Virtio Device ID,
-as indicated in section \ref{sec:Device Types}.
-Drivers for device types listed in section \ref{sec:Virtio
-Transport Options / Virtio Over PCI Bus / PCI Device Discovery}
-MUST match devices with the PCI Vendor ID 0x1AF4 and
-the Transitional PCI Device ID indicated in section
- \ref{sec:Virtio
-Transport Options / Virtio Over PCI Bus / PCI Device Discovery}.
-
-Drivers MUST match any PCI Revision ID value.
-Drivers MAY match any PCI Subsystem Vendor ID and any
-PCI Subsystem Device ID value.
-
-\subsubsection{Legacy Interfaces: A Note on PCI Device 
Discovery}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI 
Device Discovery / Legacy Interfaces: A Note on PCI Device Discovery}
-Transitional devices MUST have a PCI Revision ID of 0.
-Transitional devices MUST have the PCI Subsystem Device ID
-matching the Virtio Device ID, as indicated in section \ref{sec:Device Types}.
-Transitional devices MUST have the Transitional PCI Device ID in
-the range 0x1000 to 0x103f.
-
-This is to match legacy drivers.
-
-\subsection{PCI Device Layout}\label{sec:Virtio Transport Options / Virtio 
Over PCI Bus / PCI Device Layout}
-
-The device is configured via I/O 

[virtio-dev] [PATCH v4 3/6] transport-ccw: Split Channel IO transport to its own file

[Parav Pandit]

Place Channel IO transport specification in its own file to
better maintain it.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/157
Signed-off-by: Parav Pandit 
---
changelog:
v1->v2:
- renamed file to transport-ccw.tex
---
 content.tex   | 603 +-
 transport-ccw.tex | 601 +
 2 files changed, 602 insertions(+), 602 deletions(-)
 create mode 100644 transport-ccw.tex

diff --git a/content.tex b/content.tex
index 80c28df..cff548a 100644
--- a/content.tex
+++ b/content.tex
@@ -581,608 +581,7 @@ \chapter{Virtio Transport Options}\label{sec:Virtio 
Transport Options}
 
 \input{transport-pci.tex}
 \input{transport-mmio.tex}
-
-\section{Virtio Over Channel I/O}\label{sec:Virtio Transport Options / Virtio 
Over Channel I/O}
-
-S/390 based virtual machines support neither PCI nor MMIO, so a
-different transport is needed there.
-
-virtio-ccw uses the standard channel I/O based mechanism used for
-the majority of devices on S/390. A virtual channel device with a
-special control unit type acts as proxy to the virtio device
-(similar to the way virtio-pci uses a PCI device) and
-configuration and operation of the virtio device is accomplished
-(mostly) via channel commands. This means virtio devices are
-discoverable via standard operating system algorithms, and adding
-virtio support is mainly a question of supporting a new control
-unit type.
-
-As the S/390 is a big endian machine, the data structures transmitted
-via channel commands are big-endian: this is made clear by use of
-the types be16, be32 and be64.
-
-\subsection{Basic Concepts}\label{sec:Virtio Transport Options / Virtio over 
channel I/O / Basic Concepts}
-
-As a proxy device, virtio-ccw uses a channel-attached I/O control
-unit with a special control unit type (0x3832) and a control unit
-model corresponding to the attached virtio device's subsystem
-device ID, accessed via a virtual I/O subchannel and a virtual
-channel path of type 0x32. This proxy device is discoverable via
-normal channel subsystem device discovery (usually a STORE
-SUBCHANNEL loop) and answers to the basic channel commands:
-
-\begin{itemize}
-\item NO-OPERATION (0x03)
-\item BASIC SENSE (0x04)
-\item TRANSFER IN CHANNEL (0x08)
-\item SENSE ID (0xe4)
-\end{itemize}
-
-For a virtio-ccw proxy device, SENSE ID will return the following
-information:
-
-\begin{tabular}{ |l|l|l| }
-\hline
-Bytes & Description & Contents \\
-\hline \hline
-0 & reserved  & 0xff \\
-\hline
-1-2   & control unit type & 0x3832 \\
-\hline
-3 & control unit model&  \\
-\hline
-4-5   & device type   & zeroes (unset) \\
-\hline
-6 & device model  & zeroes (unset) \\
-\hline
-7-255 & extended SenseId data & zeroes (unset) \\
-\hline
-\end{tabular}
-
-A virtio-ccw proxy device facilitates:
-\begin{itemize} 
-\item Discovery and attachment of virtio devices (as described above).
-\item Initialization of virtqueues and transport-specific facilities (using
-  virtio-specific channel commands).
-\item Notifications (via hypercall and a combination of I/O interrupts
-  and indicator bits).
-\end{itemize} 
-
-\subsubsection{Channel Commands for Virtio}\label{sec:Virtio Transport Options 
/ Virtio
-over channel I/O / Basic Concepts/ Channel Commands for Virtio}
-
-In addition to the basic channel commands, virtio-ccw defines a
-set of channel commands related to configuration and operation of
-virtio:
-
-\begin{lstlisting}
-#define CCW_CMD_SET_VQ 0x13
-#define CCW_CMD_VDEV_RESET 0x33
-#define CCW_CMD_SET_IND 0x43
-#define CCW_CMD_SET_CONF_IND 0x53
-#define CCW_CMD_SET_IND_ADAPTER 0x73
-#define CCW_CMD_READ_FEAT 0x12
-#define CCW_CMD_WRITE_FEAT 0x11
-#define CCW_CMD_READ_CONF 0x22
-#define CCW_CMD_WRITE_CONF 0x21
-#define CCW_CMD_WRITE_STATUS 0x31
-#define CCW_CMD_READ_VQ_CONF 0x32
-#define CCW_CMD_SET_VIRTIO_REV 0x83
-#define CCW_CMD_READ_STATUS 0x72
-\end{lstlisting}
-
-\subsubsection{Notifications}\label{sec:Virtio Transport Options / Virtio
-over channel I/O / Basic Concepts/ Notifications}
-
-Available buffer notifications are realized as a hypercall. No additional
-setup by the driver is needed. The operation of available buffer
-notifications is described in section \ref{sec:Virtio Transport Options /
-Virtio over channel I/O / Device Operation / Guest->Host Notification}.
-
-Used buffer notifications are realized either as so-called classic or
-adapter I/O interrupts depending on a transport level negotiation. The
-initialization is described in sections \ref{sec:Virtio Transport Options
-/ Virtio over channel I/O / Device Initialization / Setting Up Indicators
-/ Setting Up Classic Queue Indicators} and \ref{sec:Virtio Transport
-Options / Virtio over channel I/O / Device Initialization / Setting Up
-Indicators / Setting Up Two-Stage Queue Indicators} respectively.  The
-operation of each flavor is described in sections \ref{sec:Virtio
-Transport Options / 

[virtio-dev] [PATCH v4 2/6] transport-mmio: Split MMIO transport to its own file

[Parav Pandit]

Place MMIO transport specification in its own file to better maintain it.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/157
Signed-off-by: Parav Pandit 
---
 content.tex| 554 +
 transport-mmio.tex | 552 
 2 files changed, 553 insertions(+), 553 deletions(-)
 create mode 100644 transport-mmio.tex

diff --git a/content.tex b/content.tex
index be911e6..80c28df 100644
--- a/content.tex
+++ b/content.tex
@@ -580,559 +580,7 @@ \chapter{Virtio Transport Options}\label{sec:Virtio 
Transport Options}
 into virtio general and bus-specific sections.
 
 \input{transport-pci.tex}
-
-\subsection{MMIO Device Discovery}\label{sec:Virtio Transport Options / Virtio 
Over MMIO / MMIO Device Discovery}
-
-Unlike PCI, MMIO provides no generic device discovery mechanism.  For each
-device, the guest OS will need to know the location of the registers
-and interrupt(s) used.  The suggested binding for systems using
-flattened device trees is shown in this example:
-
-\begin{lstlisting}
-// EXAMPLE: virtio_block device taking 512 bytes at 0x1e000, interrupt 42.
-virtio_block@1e000 {
-compatible = "virtio,mmio";
-reg = <0x1e000 0x200>;
-interrupts = <42>;
-}
-\end{lstlisting}
-
-\subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / 
Virtio Over MMIO / MMIO Device Register Layout}
-
-MMIO virtio devices provide a set of memory mapped control
-registers followed by a device-specific configuration space,
-described in the table~\ref{tab:Virtio Trasport Options / Virtio Over MMIO / 
MMIO Device Register Layout}.
-
-All register values are organized as Little Endian.
-
-\newcommand{\mmioreg}[5]{% Name Function Offset Direction Description
-  {\field{#1}} \newline #3 \newline #4 & {\bf#2} \newline #5 \\
-}
-
-\newcommand{\mmiodreg}[7]{% NameHigh NameLow Function OffsetHigh OffsetLow 
Direction Description
-  {\field{#1}} \newline #4 \newline {\field{#2}} \newline #5 \newline #6 & 
{\bf#3} \newline #7 \\
-}
-
-\begin{longtable}{p{0.2\textwidth}p{0.7\textwidth}}
-  \caption {MMIO Device Register Layout}
-  \label{tab:Virtio Trasport Options / Virtio Over MMIO / MMIO Device Register 
Layout} \\
-  \hline
-  \mmioreg{Name}{Function}{Offset from base}{Direction}{Description} 
-  \hline 
-  \hline 
-  \endfirsthead
-  \hline
-  \mmioreg{Name}{Function}{Offset from the base}{Direction}{Description} 
-  \hline 
-  \hline 
-  \endhead
-  \endfoot
-  \endlastfoot
-  \mmioreg{MagicValue}{Magic value}{0x000}{R}{%
-0x74726976
-(a Little Endian equivalent of the ``virt'' string).
-  } 
-  \hline
-  \mmioreg{Version}{Device version number}{0x004}{R}{%
-0x2.
-\begin{note}
-  Legacy devices (see \ref{sec:Virtio Transport Options / Virtio Over MMIO 
/ Legacy interface}~\nameref{sec:Virtio Transport Options / Virtio Over MMIO / 
Legacy interface}) used 0x1.
-\end{note}
-  }
-  \hline 
-  \mmioreg{DeviceID}{Virtio Subsystem Device ID}{0x008}{R}{%
-See \ref{sec:Device Types}~\nameref{sec:Device Types} for possible values.
-Value zero (0x0) is used to
-define a system memory map with placeholder devices at static,
-well known addresses, assigning functions to them depending
-on user's needs.
-  }
-  \hline 
-  \mmioreg{VendorID}{Virtio Subsystem Vendor ID}{0x00c}{R}{}
-  \hline 
-  \mmioreg{DeviceFeatures}{Flags representing features the device 
supports}{0x010}{R}{%
-Reading from this register returns 32 consecutive flag bits,
-the least significant bit depending on the last value written to
-\field{DeviceFeaturesSel}. Access to this register returns
-bits $\field{DeviceFeaturesSel}*32$ to 
$(\field{DeviceFeaturesSel}*32)+31$, eg.
-feature bits 0 to 31 if \field{DeviceFeaturesSel} is set to 0 and
-features bits 32 to 63 if \field{DeviceFeaturesSel} is set to 1.
-Also see \ref{sec:Basic Facilities of a Virtio Device / Feature 
Bits}~\nameref{sec:Basic Facilities of a Virtio Device / Feature Bits}.
-  }
-  \hline 
-  \mmioreg{DeviceFeaturesSel}{Device (host) features word 
selection.}{0x014}{W}{%
-Writing to this register selects a set of 32 device feature bits
-accessible by reading from \field{DeviceFeatures}.
-  }
-  \hline 
-  \mmioreg{DriverFeatures}{Flags representing device features understood and 
activated by the driver}{0x020}{W}{%
-Writing to this register sets 32 consecutive flag bits, the least 
significant
-bit depending on the last value written to \field{DriverFeaturesSel}.
- Access to this register sets bits $\field{DriverFeaturesSel}*32$
-to $(\field{DriverFeaturesSel}*32)+31$, eg. feature bits 0 to 31 if
-\field{DriverFeaturesSel} is set to 0 and features bits 32 to 63 if
-\field{DriverFeaturesSel} is set to 1. Also see \ref{sec:Basic Facilities 
of a Virtio Device / Feature Bits}~\nameref{sec:Basic Facilities of a Virtio 
Device / Feature Bits}.
-  }
-  \hline 
-  

[virtio-dev] [PATCH v4 5/6] transport-mmio: Fix spellings and white spaces

[Parav Pandit]

Now that we have individual files, fix reported spelling errors.

While at it, remove trailing white spaces.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/157
Signed-off-by: Parav Pandit 
---
changelog:
v0->v1:
- removed many trailing white spaces
---
 transport-mmio.tex | 90 +++---
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/transport-mmio.tex b/transport-mmio.tex
index 7f2e0c3..65bae54 100644
--- a/transport-mmio.tex
+++ b/transport-mmio.tex
@@ -18,7 +18,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 
 MMIO virtio devices provide a set of memory mapped control
 registers followed by a device-specific configuration space,
-described in the table~\ref{tab:Virtio Trasport Options / Virtio Over MMIO / 
MMIO Device Register Layout}.
+described in the table~\ref{tab:Virtio Transport Options / Virtio Over MMIO / 
MMIO Device Register Layout}.
 
 All register values are organized as Little Endian.
 
@@ -32,23 +32,23 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 
 \begin{longtable}{p{0.2\textwidth}p{0.7\textwidth}}
   \caption {MMIO Device Register Layout}
-  \label{tab:Virtio Trasport Options / Virtio Over MMIO / MMIO Device Register 
Layout} \\
+  \label{tab:Virtio Transport Options / Virtio Over MMIO / MMIO Device 
Register Layout} \\
+  \hline
+  \mmioreg{Name}{Function}{Offset from base}{Direction}{Description}
+  \hline
   \hline
-  \mmioreg{Name}{Function}{Offset from base}{Direction}{Description} 
-  \hline 
-  \hline 
   \endfirsthead
   \hline
-  \mmioreg{Name}{Function}{Offset from the base}{Direction}{Description} 
-  \hline 
-  \hline 
+  \mmioreg{Name}{Function}{Offset from the base}{Direction}{Description}
+  \hline
+  \hline
   \endhead
   \endfoot
   \endlastfoot
   \mmioreg{MagicValue}{Magic value}{0x000}{R}{%
 0x74726976
 (a Little Endian equivalent of the ``virt'' string).
-  } 
+  }
   \hline
   \mmioreg{Version}{Device version number}{0x004}{R}{%
 0x2.
@@ -56,7 +56,7 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
   Legacy devices (see \ref{sec:Virtio Transport Options / Virtio Over MMIO 
/ Legacy interface}~\nameref{sec:Virtio Transport Options / Virtio Over MMIO / 
Legacy interface}) used 0x1.
 \end{note}
   }
-  \hline 
+  \hline
   \mmioreg{DeviceID}{Virtio Subsystem Device ID}{0x008}{R}{%
 See \ref{sec:Device Types}~\nameref{sec:Device Types} for possible values.
 Value zero (0x0) is used to
@@ -64,9 +64,9 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 well known addresses, assigning functions to them depending
 on user's needs.
   }
-  \hline 
+  \hline
   \mmioreg{VendorID}{Virtio Subsystem Vendor ID}{0x00c}{R}{}
-  \hline 
+  \hline
   \mmioreg{DeviceFeatures}{Flags representing features the device 
supports}{0x010}{R}{%
 Reading from this register returns 32 consecutive flag bits,
 the least significant bit depending on the last value written to
@@ -76,12 +76,12 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 features bits 32 to 63 if \field{DeviceFeaturesSel} is set to 1.
 Also see \ref{sec:Basic Facilities of a Virtio Device / Feature 
Bits}~\nameref{sec:Basic Facilities of a Virtio Device / Feature Bits}.
   }
-  \hline 
+  \hline
   \mmioreg{DeviceFeaturesSel}{Device (host) features word 
selection.}{0x014}{W}{%
 Writing to this register selects a set of 32 device feature bits
 accessible by reading from \field{DeviceFeatures}.
   }
-  \hline 
+  \hline
   \mmioreg{DriverFeatures}{Flags representing device features understood and 
activated by the driver}{0x020}{W}{%
 Writing to this register sets 32 consecutive flag bits, the least 
significant
 bit depending on the last value written to \field{DriverFeaturesSel}.
@@ -90,41 +90,41 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio 
Transport Options / Vi
 \field{DriverFeaturesSel} is set to 0 and features bits 32 to 63 if
 \field{DriverFeaturesSel} is set to 1. Also see \ref{sec:Basic Facilities 
of a Virtio Device / Feature Bits}~\nameref{sec:Basic Facilities of a Virtio 
Device / Feature Bits}.
   }
-  \hline 
+  \hline
   \mmioreg{DriverFeaturesSel}{Activated (guest) features word 
selection}{0x024}{W}{%
 Writing to this register selects a set of 32 activated feature
 bits accessible by writing to \field{DriverFeatures}.
   }
-  \hline 
+  \hline
   \mmioreg{QueueSel}{Virtual queue index}{0x030}{W}{%
 Writing to this register selects the virtual queue that the
 following operations on \field{QueueNumMax}, \field{QueueNum}, 
\field{QueueReady},
 \field{QueueDescLow}, \field{QueueDescHigh}, \field{QueueDriverlLow}, 
\field{QueueDriverHigh},
 \field{QueueDeviceLow}, \field{QueueDeviceHigh} and \field{QueueReset} 
apply to. The index
-number of the first 

[virtio-dev] [PATCH v4 0/6] Split transport specific files

[Parav Pandit]

Problem statement:
Currently several tens of pages worth of transport specification
is situated in single file. Many parts of the specification
are already maintained in separate files.

Such separate files enables better maintenance of the specification overall.

Solution:
Follow the approach similar to rest of the spec, and keep
transport specific spec in their individual files.

This series is for fixing issue [1].

[1] Fixes: https://github.com/oasis-tcs/virtio-spec/issues/157

Please review.

Patch summary:
patch 1 moves pci transport to its own file
patch 2 moves mmio transport to its own file
patch 3 moves channel io transport to its own file
patch 4 to 6 fixes spelling errors existed in the spec which are
discovered during splitting the files

changelog:
v3->v4:
- added github issue number fixes tag to patches 4 to 6
v2->v3:
- updated subject for patches 4 to 6
v1->v2:
- renamed transport-channelio.tex to transport-ccw.tex
- removed trailing white spaces in transport-ccw.tex


Parav Pandit (6):
  transport-pci: Split PCI transport to its own file
  transport-mmio: Split MMIO transport to its own file
  transport-ccw: Split Channel IO transport to its own file
  transport-pci: Fix spellings and white spaces
  transport-mmio: Fix spellings and white spaces
  transport-ccw: Fix spellings and white spaces

 content.tex| 2318 +---
 transport-ccw.tex  |  601 
 transport-mmio.tex |  552 +++
 transport-pci.tex  | 1160 ++
 4 files changed, 2316 insertions(+), 2315 deletions(-)
 create mode 100644 transport-ccw.tex
 create mode 100644 transport-mmio.tex
 create mode 100644 transport-pci.tex

-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v9] virtio-net: support inner header hash

[Jason Wang]

Hi:

在 2023/2/22 14:46, Heng Qi 写道:

Hi, Jason. Long time no see. :)

在 2023/2/22 上午11:22, Jason Wang 写道:


在 2023/2/22 01:50, Michael S. Tsirkin 写道:

On Sat, Feb 18, 2023 at 10:37:15PM +0800, Heng Qi wrote:

+\subparagraph{Security risks between encapsulated packets and RSS}
+There may be potential security risks when encapsulated packets 
using RSS to
+select queues for placement. When a user inside a tunnel tries to 
control the



What do you mean by "user" here? Is it a remote or local one?



I mean a remote attacker who is not under the control of the tunnel 
owner.



Anything may the tunnel different? I think this can happen even without 
tunnel (and even with single queue).


How to mitigate those attackers seems more like a implementation details 
where might require fair queuing or other QOS technology which has been 
well studied.


It seems out of the scope of the spec (unless we want to let driver 
manageable QOS).


Thanks




Thanks.



+enqueuing of encapsulated packets, then the user can flood the 
device with invaild
+packets, and the flooded packets may be hashed into the same queue 
as packets in

+other normal tunnels, which causing the queue to overflow.
+
+This can pose several security risks:
+\begin{itemize}
+\item  Encapsulated packets in the normal tunnels cannot be 
enqueued due to queue

+   overflow, resulting in a large amount of packet loss.
+\item  The delay and retransmission of packets in the normal 
tunnels are extremely increased.
+\item  The user can observe the traffic information and enqueue 
information of other normal

+   tunnels, and conduct targeted DoS attacks.
+\end{\itemize}
+

Hmm with this all written out it sounds pretty severe.



I think we need first understand whether or not it's a problem that 
we need to solve at spec level:


1) anything make encapsulated packets different or why we can't hit 
this problem without encapsulation


2) whether or not it's the implementation details that the spec 
doesn't need to care (or how it is solved in real NIC)


Thanks



At this point with no ways to mitigate, I don't feel this is something
e.g. Linux can enable.  I am not going to nack the spec patch if
others  find this somehow useful e.g. for dpdk.
How about CC e.g. dpdk devs or whoever else is going to use this
and asking them for the opinion?







-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v4 1/2] virtio-net: Describe dev cfg fields read only

[Parav Pandit]

Device configuration fields are read only. Avoid duplicating this
description for multiple fields.

Instead describe it one time and do it in the driver requirements
section.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
Reviewed-by: David Edmondson 
Signed-off-by: Parav Pandit 
---
changelog:
v3->v4:
- write driver requirement as normative statement
- add back read only wording in the description
v2->v3:
- split as new patch
---
 device-types/net/description.tex | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 73501b6..821f7b0 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -156,25 +156,28 @@ \subsubsection{Legacy Interface: Feature 
bits}\label{sec:Device Types / Network
 \subsection{Device configuration layout}\label{sec:Device Types / Network 
Device / Device configuration layout}
 \label{sec:Device Types / Block Device / Feature bits / Device configuration 
layout}
 
-Device configuration fields are listed below, they are read-only for a driver. 
The \field{mac} address field
-always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
-\field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
-read-only bits (for the driver) are currently defined for the status field:
-VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
+Device configuration fields are listed below. All the device
+configuration fields are read-only for the driver.
+
+The \field{mac} address field always exists (though is only
+valid if VIRTIO_NET_F_MAC is set), and \field{status} only
+exists if VIRTIO_NET_F_STATUS is set. Two bits are currently
+defined for the status field: VIRTIO_NET_S_LINK_UP and
+VIRTIO_NET_S_ANNOUNCE.
 
 \begin{lstlisting}
 #define VIRTIO_NET_S_LINK_UP 1
 #define VIRTIO_NET_S_ANNOUNCE2
 \end{lstlisting}
 
-The following driver-read-only field, \field{max_virtqueue_pairs} only exists 
if
+The following field, \field{max_virtqueue_pairs} only exists if
 VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is set. This field specifies the maximum 
number
 of each of transmit and receive virtqueues (receiveq1\ldots receiveqN
 and transmitq1\ldots transmitqN respectively) that can be configured once at 
least one of these features
 is negotiated.
 
-The following driver-read-only field, \field{mtu} only exists if
-VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
+The following field, \field{mtu} only exists if VIRTIO_NET_F_MTU
+is set. This field specifies the maximum MTU for the driver to
 use.
 
 The following two fields, \field{speed} and \field{duplex}, only
@@ -264,6 +267,8 @@ \subsection{Device configuration layout}\label{sec:Device 
Types / Network Device
 
 \drivernormative{\subsubsection}{Device configuration layout}{Device Types / 
Network Device / Device configuration layout}
 
+The driver MUST NOT write to any of the device configuration fields.
+
 A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
 If the driver negotiates the VIRTIO_NET_F_MAC feature, the driver MUST set
 the physical address of the NIC to \field{mac}.  Otherwise, it SHOULD
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v4 2/2] virtio-net: Define cfg fields before description

[Parav Pandit]

Currently some fields of the virtio_net_config structure are defined
before introducing the structure and some are defined after
introducing virtio_net_config.
Better to define the configuration layout first followed by
description of all the fields.

Device configuration fields are described in the section. Change wording
from 'listed' to 'described' as suggested in patch [1].

[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg4.html

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/161
Reviewed-by: David Edmondson 
Signed-off-by: Parav Pandit 
---
changelog:
v2->v3:
- split the patch for read only description as prepration patch
- rebased
v1->v2:
- remove read-only wording from multiple places
v0->v1:
- Change wording about device configuration field introduction
- remove duplicate read-only wording for status field
- reword sentence to read it better
---
 device-types/net/description.tex | 42 +---
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 821f7b0..7033594 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -156,14 +156,29 @@ \subsubsection{Legacy Interface: Feature 
bits}\label{sec:Device Types / Network
 \subsection{Device configuration layout}\label{sec:Device Types / Network 
Device / Device configuration layout}
 \label{sec:Device Types / Block Device / Feature bits / Device configuration 
layout}
 
-Device configuration fields are listed below. All the device
-configuration fields are read-only for the driver.
+The network device has the following device configuration layout. All the
+device configuration fields are read-only for the driver.
 
-The \field{mac} address field always exists (though is only
-valid if VIRTIO_NET_F_MAC is set), and \field{status} only
-exists if VIRTIO_NET_F_STATUS is set. Two bits are currently
-defined for the status field: VIRTIO_NET_S_LINK_UP and
-VIRTIO_NET_S_ANNOUNCE.
+\begin{lstlisting}
+struct virtio_net_config {
+u8 mac[6];
+le16 status;
+le16 max_virtqueue_pairs;
+le16 mtu;
+le32 speed;
+u8 duplex;
+u8 rss_max_key_size;
+le16 rss_max_indirection_table_length;
+le32 supported_hash_types;
+};
+\end{lstlisting}
+
+The \field{mac} address field always exists (although it is only
+valid if VIRTIO_NET_F_MAC is set).
+
+The \field{status} only exists if VIRTIO_NET_F_STATUS is set.
+Two bits are currently defined for the status field: VIRTIO_NET_S_LINK_UP
+and VIRTIO_NET_S_ANNOUNCE.
 
 \begin{lstlisting}
 #define VIRTIO_NET_S_LINK_UP 1
@@ -193,19 +208,6 @@ \subsection{Device configuration layout}\label{sec:Device 
Types / Network Device
 is expected to re-read these values after receiving a
 configuration change notification.
 
-\begin{lstlisting}
-struct virtio_net_config {
-u8 mac[6];
-le16 status;
-le16 max_virtqueue_pairs;
-le16 mtu;
-le32 speed;
-u8 duplex;
-u8 rss_max_key_size;
-le16 rss_max_indirection_table_length;
-le32 supported_hash_types;
-};
-\end{lstlisting}
 The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS 
or VIRTIO_NET_F_HASH_REPORT is set.
 It specifies the maximum supported length of RSS key in bytes.
 
-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v4 0/2] virtio-net: Improve dev config layout

[Parav Pandit]

This two patches improve dev configuration layout in two ways.

1. Define device configuration layout before describing it fields
2. Avoid duplicate description of its read only fields and whole
layout

Patch summary:
patch-1: consolidate read only field at one place in driver
requirements
patch-2: define device configuration layout before describing its
fields.

changelog:
v3->v4:
- wrote driver requirement as normative statement
- moved read only line to description section
v2->v3:
- split into two patches
- move read only description to driver requirements section

Parav Pandit (2):
  virtio-net: Describe dev cfg fields read only
  virtio-net: Define cfg fields before description

 device-types/net/description.tex | 49 ++--
 1 file changed, 28 insertions(+), 21 deletions(-)

-- 
2.26.2


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] RE: [PATCH v6] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

Hi, Parav.

在 2023/2/22 下午10:28, Parav Pandit 写道:



From: Michael S. Tsirkin 
Sent: Wednesday, February 22, 2023 1:29 AM

MMIO device has vq_index register too.
I am inclined to vq_index given current state of spec

[mst@tuck virtio]$ git grep vq_index|wc -l
0


:)

Grep for "virtqueue index".
We need to change "virtqueue index" to "virtqueue number".



Yes, this is more unified in the spec. :)

This version is closed, please help review the v7 version [1] if you 
have time.


[1] https://lists.oasis-open.org/archives/virtio-dev/202302/msg00520.html

Thanks very much for your response!




and new additions by Michael.

admin vq? Good point, I will change that to vqn too.

Ok. sounds good.


Michael,
Can you please suggest vqn or vq_index to use?

[mst@tuck virtio]$ git grep vqn|wc -l
5

I'd say vqn has precedent.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org



-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] RE: [PATCH v6] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Michael S. Tsirkin 
> Sent: Wednesday, February 22, 2023 1:29 AM

> > MMIO device has vq_index register too.
> > I am inclined to vq_index given current state of spec
> 
> [mst@tuck virtio]$ git grep vq_index|wc -l
> 0
> 
:)

Grep for "virtqueue index".
We need to change "virtqueue index" to "virtqueue number".

> > and new additions by Michael.
> 
> admin vq? Good point, I will change that to vqn too.

Ok. sounds good.

> 
> > Michael,
> > Can you please suggest vqn or vq_index to use?
> 
> [mst@tuck virtio]$ git grep vqn|wc -l
> 5
> 
> I'd say vqn has precedent.
> 

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Cornelia Huck]

On Wed, Feb 22 2023, "Michael S. Tsirkin"  wrote:

> On Wed, Feb 22, 2023 at 01:07:07PM +0100, Cornelia Huck wrote:
>> On Wed, Feb 22 2023, "Michael S. Tsirkin"  wrote:
>> 
>> > On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
>> >> On Tue, Feb 21 2023, "Michael S. Tsirkin"  wrote:
>> >> 
>> >> > On Tue, Feb 21, 2023 at 05:59:52PM +, Parav Pandit wrote:
>> >> >> 
>> >> >> > From: Michael S. Tsirkin 
>> >> >> > Sent: Tuesday, February 21, 2023 12:52 PM
>> >> >> > 
>> >> >> > On Tue, Feb 21, 2023 at 05:50:09PM +, Parav Pandit wrote:
>> >> >> > > Hence, it should be mentioned as read-only fields, so when the 
>> >> >> > > driver writes
>> >> >> > something to read-only fields, it can be considered as undefined 
>> >> >> > behavior on
>> >> >> > such fields.
>> >> >> > >
>> >> >> > 
>> >> >> > In the description not in the normative statements. normative 
>> >> >> > sections just tell
>> >> >> > driver what it must and must not do, in the standard RFC terms.
>> >> >> > 
>> >> >> Got it.
>> >> >> I will shift them as read-only in the description section.
>> >> >> And normative in the device and driver section.
>> >> >> Device section:
>> >> >> Any writes to config space fields is ignored by the device, because 
>> >> >> these are read-only fields for the driver.
>> >> >
>> >> > writes is plural so "are ignored"
>> >> >
>> >> > but more importantly use rfc terms in normative sections.
>> >> 
>> >> I don't think you need to talk about "read-only" in the normative
>> >> sections (that belongs to the descriptive sections.) I'd use
>> >> 
>> >> "The device MUST ignore any writes to config space fields by the
>> >> driver."
>> >
>> > Hmm. Is this something we previously required for read only fields?
>> 
>> So, better make it SHOULD?
>> 
>> (The only alternative to ignoring I see is breaking the device, and I
>> think ignoring is preferable.)
>
> We can just skip adding a new requirement completely - we'll never get
> there with a compliant driver.  This is what we do e.g. for MMIO.
> Why not?

That would be fine with me as well.

> This has an advantage as this allows backing config with regular RAM.
> Also I feel that since it always said "read only for driver" then
> this implies a restriction on driver not the device.

Indeed. The normative statement below should be enough to make that
"read-only for the driver" thing obvious.

>
>> >
>> >
>> >> >
>> >> >> 
>> >> >> Driver section:
>> >> >> Driver must not write to read-only fields.
>> >> 
>> >> "The driver MUST NOT write to any config space field."


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 01:07:07PM +0100, Cornelia Huck wrote:
> On Wed, Feb 22 2023, "Michael S. Tsirkin"  wrote:
> 
> > On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
> >> On Tue, Feb 21 2023, "Michael S. Tsirkin"  wrote:
> >> 
> >> > On Tue, Feb 21, 2023 at 05:59:52PM +, Parav Pandit wrote:
> >> >> 
> >> >> > From: Michael S. Tsirkin 
> >> >> > Sent: Tuesday, February 21, 2023 12:52 PM
> >> >> > 
> >> >> > On Tue, Feb 21, 2023 at 05:50:09PM +, Parav Pandit wrote:
> >> >> > > Hence, it should be mentioned as read-only fields, so when the 
> >> >> > > driver writes
> >> >> > something to read-only fields, it can be considered as undefined 
> >> >> > behavior on
> >> >> > such fields.
> >> >> > >
> >> >> > 
> >> >> > In the description not in the normative statements. normative 
> >> >> > sections just tell
> >> >> > driver what it must and must not do, in the standard RFC terms.
> >> >> > 
> >> >> Got it.
> >> >> I will shift them as read-only in the description section.
> >> >> And normative in the device and driver section.
> >> >> Device section:
> >> >> Any writes to config space fields is ignored by the device, because 
> >> >> these are read-only fields for the driver.
> >> >
> >> > writes is plural so "are ignored"
> >> >
> >> > but more importantly use rfc terms in normative sections.
> >> 
> >> I don't think you need to talk about "read-only" in the normative
> >> sections (that belongs to the descriptive sections.) I'd use
> >> 
> >> "The device MUST ignore any writes to config space fields by the
> >> driver."
> >
> > Hmm. Is this something we previously required for read only fields?
> 
> So, better make it SHOULD?
> 
> (The only alternative to ignoring I see is breaking the device, and I
> think ignoring is preferable.)

We can just skip adding a new requirement completely - we'll never get
there with a compliant driver.  This is what we do e.g. for MMIO.
Why not?
This has an advantage as this allows backing config with regular RAM.
Also I feel that since it always said "read only for driver" then
this implies a restriction on driver not the device.

> >
> >
> >> >
> >> >> 
> >> >> Driver section:
> >> >> Driver must not write to read-only fields.
> >> 
> >> "The driver MUST NOT write to any config space field."


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Cornelia Huck]

On Wed, Feb 22 2023, "Michael S. Tsirkin"  wrote:

> On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
>> On Tue, Feb 21 2023, "Michael S. Tsirkin"  wrote:
>> 
>> > On Tue, Feb 21, 2023 at 05:59:52PM +, Parav Pandit wrote:
>> >> 
>> >> > From: Michael S. Tsirkin 
>> >> > Sent: Tuesday, February 21, 2023 12:52 PM
>> >> > 
>> >> > On Tue, Feb 21, 2023 at 05:50:09PM +, Parav Pandit wrote:
>> >> > > Hence, it should be mentioned as read-only fields, so when the driver 
>> >> > > writes
>> >> > something to read-only fields, it can be considered as undefined 
>> >> > behavior on
>> >> > such fields.
>> >> > >
>> >> > 
>> >> > In the description not in the normative statements. normative sections 
>> >> > just tell
>> >> > driver what it must and must not do, in the standard RFC terms.
>> >> > 
>> >> Got it.
>> >> I will shift them as read-only in the description section.
>> >> And normative in the device and driver section.
>> >> Device section:
>> >> Any writes to config space fields is ignored by the device, because these 
>> >> are read-only fields for the driver.
>> >
>> > writes is plural so "are ignored"
>> >
>> > but more importantly use rfc terms in normative sections.
>> 
>> I don't think you need to talk about "read-only" in the normative
>> sections (that belongs to the descriptive sections.) I'd use
>> 
>> "The device MUST ignore any writes to config space fields by the
>> driver."
>
> Hmm. Is this something we previously required for read only fields?

So, better make it SHOULD?

(The only alternative to ignoring I see is breaking the device, and I
think ignoring is preferable.)

>
>
>> >
>> >> 
>> >> Driver section:
>> >> Driver must not write to read-only fields.
>> 
>> "The driver MUST NOT write to any config space field."


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 10:01:24AM +0100, Cornelia Huck wrote:
> On Tue, Feb 21 2023, "Michael S. Tsirkin"  wrote:
> 
> > On Tue, Feb 21, 2023 at 05:59:52PM +, Parav Pandit wrote:
> >> 
> >> > From: Michael S. Tsirkin 
> >> > Sent: Tuesday, February 21, 2023 12:52 PM
> >> > 
> >> > On Tue, Feb 21, 2023 at 05:50:09PM +, Parav Pandit wrote:
> >> > > Hence, it should be mentioned as read-only fields, so when the driver 
> >> > > writes
> >> > something to read-only fields, it can be considered as undefined 
> >> > behavior on
> >> > such fields.
> >> > >
> >> > 
> >> > In the description not in the normative statements. normative sections 
> >> > just tell
> >> > driver what it must and must not do, in the standard RFC terms.
> >> > 
> >> Got it.
> >> I will shift them as read-only in the description section.
> >> And normative in the device and driver section.
> >> Device section:
> >> Any writes to config space fields is ignored by the device, because these 
> >> are read-only fields for the driver.
> >
> > writes is plural so "are ignored"
> >
> > but more importantly use rfc terms in normative sections.
> 
> I don't think you need to talk about "read-only" in the normative
> sections (that belongs to the descriptive sections.) I'd use
> 
> "The device MUST ignore any writes to config space fields by the
> driver."

Hmm. Is this something we previously required for read only fields?


> >
> >> 
> >> Driver section:
> >> Driver must not write to read-only fields.
> 
> "The driver MUST NOT write to any config space field."


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH v6] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 04:32:56PM +0800, Heng Qi wrote:
> 
> Hi, Alvaro.
> 
> 在 2023/2/22 下午4:09, Alvaro Karsz 写道:
> > Hi Heng,
> > 
> > > Currently, coalescing parameters are grouped for all transmit and receive
> > > virtqueues. This patch supports setting or getting the parameters for a
> > > specified virtqueue, and a typical application of this function is 
> > > netdim[1].
> > > 
> > > When the traffic between virtqueues is unbalanced, for example, one 
> > > virtqueue
> > > is busy and another virtqueue is idle, then it will be very useful to
> > > control coalescing parameters at the virtqueue granularity.
> > > 
> > > [1] https://docs.kernel.org/networking/net_dim.html
> > > 
> > > Signed-off-by: Heng Qi 
> > > Reviewed-by: Xuan Zhuo 
> > > ---
> > > This patch is on top of Alvaro's latest v7 patch: 
> > > https://lists.oasis-open.org/archives/virtio-dev/202302/msg00431.html .
> > > 
> > > v5->v6:
> > > 1. Explain that the device may set a different value than the one 
> > > passed in by the driver. @David Edmondson
> > > 
> > > v4->v5:
> > > 1. Add the correspondence between virtio_net_ctrl_coal and 
> > > virtio_net_ctrl_coal_vq and control commands. @Michael S. Tsirkin
> > > 2. Add read and write attributes for each field. @Michael S. 
> > > Tsirkin
> > > 3. A clearer description of how to set coalescing parameters for 
> > > vq reset. @Michael S. Tsirkin
> > > 4. Fix some syntax errors. @Michael S. Tsirkin, @David Edmondson
> > > 
> > > v3->v4:
> > > 1. Include virtio_net_ctrl_coal in the virtio_net_ctrl_coal_vq 
> > > structure. @Alvaro Karsz
> > > 2. Add consideration of vq reset. @Michael S. Tsirkin, @Parav 
> > > Pandit, @Alvaro Karsz
> > > 3. Avoid too many examples by giving a comprehensive example. 
> > > @Michael S. Tsirkin
> > > 4. Fix typos and streamline clarifications. @Michael S. Tsirkin, 
> > > @Parav Pandit, @Alvaro Karsz
> > > 
> > > v2->v3:
> > > 1. Add the netdim link. @Parav Pandit
> > > 2. VIRTIO_NET_F_VQ_NOTF_COAL no longer depends on 
> > > VIRTIO_NET_F_NOTF_COAL. @Michael S. Tsirkin, @Alvaro Karsz
> > > 3. _VQ_GET is explained more. @Michael S. Tsirkin
> > > 4. Add more examples to avoid misunderstandings. @Michael S. 
> > > Tsirkin
> > > 5. Clarify some statements. @Michael S. Tsirkin, @Parav Pandit, 
> > > @Alvaro Karsz
> > > 6. Adjust the virtio_net_ctrl_coal_vq structure. @Michael S. 
> > > Tsirkin
> > > 7. Fix some typos. @Michael S. Tsirkin
> > > 
> > > v1->v2:
> > > 1. Rename VIRTIO_NET_F_PERQUEUE_NOTF_COAL to 
> > > VIRTIO_NET_F_VQ_NOTF_COAL. @Michael S. Tsirkin
> > > 2. Use the \field{vqn} instead of the qid. @Michael S. Tsirkin
> > > 3. Unify tx and rx control structres into one structure 
> > > virtio_net_ctrl_coal_vq. @Michael S. Tsirkin
> > > 4. Add a new control command VIRTIO_NET_CTRL_NOTF_COAL_VQ. 
> > > @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
> > > 5. The special value 0xFFF is removed because 
> > > VIRTIO_NET_CTRL_NOTF_COAL can be used. @Alvaro Karsz
> > > 6. Clarify some special scenarios. @Michael S. Tsirkin, @Parav 
> > > Pandit, @Alvaro Karsz
> > > 
> > >   device-types/net/description.tex | 100 +--
> > >   1 file changed, 95 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/device-types/net/description.tex 
> > > b/device-types/net/description.tex
> > > index e71e33b..0d9f4b9 100644
> > > --- a/device-types/net/description.tex
> > > +++ b/device-types/net/description.tex
> > > @@ -83,6 +83,8 @@ \subsection{Feature bits}\label{sec:Device Types / 
> > > Network Device / Feature bits
> > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > >   channel.
> > > 
> > > +\item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue 
> > > notification coalescing.
> > > +
> > >   \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications 
> > > coalescing.
> > > 
> > >   \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> > > @@ -139,6 +141,7 @@ \subsubsection{Feature bit 
> > > requirements}\label{sec:Device Types / Network Device
> > >   \item[VIRTIO_NET_F_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
> > >   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or 
> > > VIRTIO_NET_F_HOST_TSO6.
> > >   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > +\item[VIRTIO_NET_F_VQ_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
> > >   \end{description}
> > > 
> > >   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / 
> > > Network Device / Feature bits / Legacy Interface: Feature bits}
> > > @@ -1508,6 +1511,14 @@ \subsubsection{Control Virtqueue}\label{sec:Device 
> > > Types / Network Device / Devi
> > >   If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
> > >   send control commands for dynamically changing the coalescing 
> > > 

[virtio-dev] Re: [PATCH v9] virtio-net: support inner header hash

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 02:46:51PM +0800, Heng Qi wrote:
> Hi, Jason. Long time no see. :)
> 
> 在 2023/2/22 上午11:22, Jason Wang 写道:
> > 
> > 在 2023/2/22 01:50, Michael S. Tsirkin 写道:
> > > On Sat, Feb 18, 2023 at 10:37:15PM +0800, Heng Qi wrote:
> > > > +\subparagraph{Security risks between encapsulated packets and RSS}
> > > > +There may be potential security risks when encapsulated packets
> > > > using RSS to
> > > > +select queues for placement. When a user inside a tunnel tries
> > > > to control the
> > 
> > 
> > What do you mean by "user" here? Is it a remote or local one?
> > 
> 
> I mean a remote attacker who is not under the control of the tunnel owner.
> 
> Thanks.

OK let's just say "remote attacker" then.

> > 
> > > > +enqueuing of encapsulated packets, then the user can flood the
> > > > device with invaild
> > > > +packets, and the flooded packets may be hashed into the same
> > > > queue as packets in
> > > > +other normal tunnels, which causing the queue to overflow.
> > > > +
> > > > +This can pose several security risks:
> > > > +\begin{itemize}
> > > > +\item  Encapsulated packets in the normal tunnels cannot be
> > > > enqueued due to queue
> > > > +   overflow, resulting in a large amount of packet loss.
> > > > +\item  The delay and retransmission of packets in the normal
> > > > tunnels are extremely increased.
> > > > +\item  The user can observe the traffic information and enqueue
> > > > information of other normal
> > > > +   tunnels, and conduct targeted DoS attacks.
> > > > +\end{\itemize}
> > > > +
> > > Hmm with this all written out it sounds pretty severe.
> > 
> > 
> > I think we need first understand whether or not it's a problem that we
> > need to solve at spec level:
> > 
> > 1) anything make encapsulated packets different or why we can't hit this
> > problem without encapsulation
> > 
> > 2) whether or not it's the implementation details that the spec doesn't
> > need to care (or how it is solved in real NIC)
> > 
> > Thanks
> > 
> > 
> > > At this point with no ways to mitigate, I don't feel this is something
> > > e.g. Linux can enable.  I am not going to nack the spec patch if
> > > others  find this somehow useful e.g. for dpdk.
> > > How about CC e.g. dpdk devs or whoever else is going to use this
> > > and asking them for the opinion?
> > > 
> > > 


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] RE: [PATCH v9] virtio-net: support inner header hash

[Michael S. Tsirkin]

On Wed, Feb 22, 2023 at 03:03:32PM +0800, Heng Qi wrote:
> 
> 
> 在 2023/2/22 下午2:21, Michael S. Tsirkin 写道:
> > On Wed, Feb 22, 2023 at 10:34:39AM +0800, Heng Qi wrote:
> > > > The user will figure out how to mitigate when such QoS is not 
> > > > available. Either to run in best-effort mode or mitigate differently.
> > > Yes, our cloud security and cloud network team will configure and use 
> > > inner
> > > hash on dpdk.
> > Sounds good. More practical for dpdk than Linux.
> > Is there a chance that when the interface is close
> > to be final, but before the vote, you post a patch to the dpdk list and
> > get some acks from the maintainers, cc virtio-dev. This way we won't
> > merge something that will then go unused?
> > That would be best - do you have a prototype?
> 
> Not yet, dpdk and the business team are waiting for our virtio
> specification, and
> they have stated as a business team that their implementation on dpdk will
> not necessarily be open sourced to the community.

Ugh so no open source implementations at all :(


> > 
> > > In fact I discussed with them the security issues between
> > > tunnels,
> > > and I will quote their solutions to tunnel attacks below, but this is a
> > > problem between the tunnels, not the introduction of inner hash.
> > > I don't think we need to focus too much on this, but I'll do my best to
> > > describe the security issues between tunnels in v10.
> > > 
> > > "
> > > This is not a problem with the inner hash, it is a general problem with 
> > > the
> > > outer hash.
> > > I communicated with our people who are doing cloud security (they are also
> > > one of the demanders of inner hash),
> > > and it is a common problem for one tunnel to attack another tunnel.
> > > 
> > > For example, there is a tunnel t1; a tunnel t2; a tunnel endpoint VTEP0, 
> > > and
> > > the vni id of t1 is id1, and the vni id of v2 is id2; a VM.
> > > 
> > > At this time, regardless of the inner hash or the outer hash, the traffic 
> > > of
> > > tunnel t1 and tunnel t2 will reach the VM through VTEP0 (whether it is a
> > > single queue or multiple queues),
> > > and may be placed on the same queue to cause queue overflow.
> > Do note (and explain in spec?) that with just an outer hash and RSS it
> > is possible to configure the tunnels to use distict queues. Impossible
> > with this interface but arguably only works for a small number of
> > tunnels anyway.
> > 
> > > # Solutions:
> > More like mitigations.
> 
> Yes, you are right.
> 
> > 
> > > 1. Some current forwarding tools such as DPDK have good forwarding
> > > performance, and it is difficult to fill up the queue;
> > Oh that's a good point. If driver is generally faster than the device
> > and queues stay away from filling up there's no DoS.
> > I'd add this to the spec.
> 
> Ok.
> 
> > 
> > > 2. or switch the attack traffic to the attack clusters;
> > What is that?
> 
> This is done by the monitoring part outside the tunnel, which is also an
> important mitigation method they mentioned
> to prevent DoS between tunnels. For example, the monitoring part cuts off,
> limits or redirects the abnormal traffic of the tunnel.

This has to be outside the device though right?
Before traffic arrives at the device.

> > 
> > > 3. or connect the traffic of different tunnels to different network card
> > > ports or network devices.
> > Not sure how this is relevant. These a distinct outer MAC - with this
> > why do we need a tunnel?
> > 
> > > 4..
> > > "


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v3 1/2] virtio-net: Describe dev cfg fields read only

[Cornelia Huck]

On Tue, Feb 21 2023, "Michael S. Tsirkin"  wrote:

> On Tue, Feb 21, 2023 at 05:59:52PM +, Parav Pandit wrote:
>> 
>> > From: Michael S. Tsirkin 
>> > Sent: Tuesday, February 21, 2023 12:52 PM
>> > 
>> > On Tue, Feb 21, 2023 at 05:50:09PM +, Parav Pandit wrote:
>> > > Hence, it should be mentioned as read-only fields, so when the driver 
>> > > writes
>> > something to read-only fields, it can be considered as undefined behavior 
>> > on
>> > such fields.
>> > >
>> > 
>> > In the description not in the normative statements. normative sections 
>> > just tell
>> > driver what it must and must not do, in the standard RFC terms.
>> > 
>> Got it.
>> I will shift them as read-only in the description section.
>> And normative in the device and driver section.
>> Device section:
>> Any writes to config space fields is ignored by the device, because these 
>> are read-only fields for the driver.
>
> writes is plural so "are ignored"
>
> but more importantly use rfc terms in normative sections.

I don't think you need to talk about "read-only" in the normative
sections (that belongs to the descriptive sections.) I'd use

"The device MUST ignore any writes to config space fields by the
driver."

>
>> 
>> Driver section:
>> Driver must not write to read-only fields.

"The driver MUST NOT write to any config space field."


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] [PATCH v2] virtio-net: define the VIRTIO_NET_F_CTRL_RX_EXTRA feature bit

[Alvaro Karsz]

The VIRTIO_NET_F_CTRL_RX_EXTRA feature bit is mentioned in the spec
since version 1.0, but it's not properly defined.

This patch defines the feature bit and defines the dependency on 
VIRTIO_NET_F_CTRL_VQ.

Since this dependency is missing in previous versions, we add it now as
a "SHOULD".

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

Signed-off-by: Alvaro Karsz 
---
v2:
- Rephrase commit log, no changes to patch body.

 device-types/net/description.tex | 8 
 1 file changed, 8 insertions(+)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 8487ccd..cf37da5 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -74,6 +74,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network 
Device / Feature bits
 
 \item[VIRTIO_NET_F_CTRL_VLAN (19)] Control channel VLAN filtering.
 
+\item[VIRTIO_NET_F_CTRL_RX_EXTRA (20)] Control channel RX extra mode support.
+
 \item[VIRTIO_NET_F_GUEST_ANNOUNCE(21)] Driver can send gratuitous
 packets.
 
@@ -259,6 +261,9 @@ \subsection{Device configuration layout}\label{sec:Device 
Types / Network Device
 The device SHOULD NOT offer VIRTIO_NET_F_HASH_REPORT if it
 does not offer VIRTIO_NET_F_CTRL_VQ.
 
+The device SHOULD NOT offer VIRTIO_NET_F_CTRL_RX_EXTRA if it
+does not offer VIRTIO_NET_F_CTRL_VQ.
+
 \drivernormative{\subsubsection}{Device configuration layout}{Device Types / 
Network Device / Device configuration layout}
 
 A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
@@ -295,6 +300,9 @@ \subsection{Device configuration layout}\label{sec:Device 
Types / Network Device
 A driver SHOULD NOT negotiate VIRTIO_NET_F_HASH_REPORT if it
 does not negotiate VIRTIO_NET_F_CTRL_VQ.
 
+A driver SHOULD NOT negotiate VIRTIO_NET_F_CTRL_RX_EXTRA if it
+does not negotiate VIRTIO_NET_F_CTRL_VQ.
+
 \subsubsection{Legacy Interface: Device configuration layout}\label{sec:Device 
Types / Network Device / Device configuration layout / Legacy Interface: Device 
configuration layout}
 \label{sec:Device Types / Block Device / Feature bits / Device configuration 
layout / Legacy Interface: Device configuration layout}
 When using the legacy interface, transitional devices and drivers
-- 
2.34.1


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH v6] virtio-net: support the virtqueue coalescing moderation

[Alvaro Karsz]

Hi Heng,

>
> Currently, coalescing parameters are grouped for all transmit and receive
> virtqueues. This patch supports setting or getting the parameters for a
> specified virtqueue, and a typical application of this function is netdim[1].
>
> When the traffic between virtqueues is unbalanced, for example, one virtqueue
> is busy and another virtqueue is idle, then it will be very useful to
> control coalescing parameters at the virtqueue granularity.
>
> [1] https://docs.kernel.org/networking/net_dim.html
>
> Signed-off-by: Heng Qi 
> Reviewed-by: Xuan Zhuo 
> ---
> This patch is on top of Alvaro's latest v7 patch: 
> https://lists.oasis-open.org/archives/virtio-dev/202302/msg00431.html .
>
> v5->v6:
>1. Explain that the device may set a different value than the one 
> passed in by the driver. @David Edmondson
>
> v4->v5:
>1. Add the correspondence between virtio_net_ctrl_coal and 
> virtio_net_ctrl_coal_vq and control commands. @Michael S. Tsirkin
>2. Add read and write attributes for each field. @Michael S. Tsirkin
>3. A clearer description of how to set coalescing parameters for vq 
> reset. @Michael S. Tsirkin
>4. Fix some syntax errors. @Michael S. Tsirkin, @David Edmondson
>
> v3->v4:
>1. Include virtio_net_ctrl_coal in the virtio_net_ctrl_coal_vq 
> structure. @Alvaro Karsz
>2. Add consideration of vq reset. @Michael S. Tsirkin, @Parav Pandit, 
> @Alvaro Karsz
>3. Avoid too many examples by giving a comprehensive example. @Michael 
> S. Tsirkin
>4. Fix typos and streamline clarifications. @Michael S. Tsirkin, 
> @Parav Pandit, @Alvaro Karsz
>
> v2->v3:
>1. Add the netdim link. @Parav Pandit
>2. VIRTIO_NET_F_VQ_NOTF_COAL no longer depends on 
> VIRTIO_NET_F_NOTF_COAL. @Michael S. Tsirkin, @Alvaro Karsz
>3. _VQ_GET is explained more. @Michael S. Tsirkin
>4. Add more examples to avoid misunderstandings. @Michael S. Tsirkin
>5. Clarify some statements. @Michael S. Tsirkin, @Parav Pandit, 
> @Alvaro Karsz
>6. Adjust the virtio_net_ctrl_coal_vq structure. @Michael S. Tsirkin
>7. Fix some typos. @Michael S. Tsirkin
>
> v1->v2:
>1. Rename VIRTIO_NET_F_PERQUEUE_NOTF_COAL to 
> VIRTIO_NET_F_VQ_NOTF_COAL. @Michael S. Tsirkin
>2. Use the \field{vqn} instead of the qid. @Michael S. Tsirkin
>3. Unify tx and rx control structres into one structure 
> virtio_net_ctrl_coal_vq. @Michael S. Tsirkin
>4. Add a new control command VIRTIO_NET_CTRL_NOTF_COAL_VQ. @Michael S. 
> Tsirkin, @Parav Pandit, @Alvaro Karsz
>5. The special value 0xFFF is removed because 
> VIRTIO_NET_CTRL_NOTF_COAL can be used. @Alvaro Karsz
>6. Clarify some special scenarios. @Michael S. Tsirkin, @Parav Pandit, 
> @Alvaro Karsz
>
>  device-types/net/description.tex | 100 +--
>  1 file changed, 95 insertions(+), 5 deletions(-)
>
> diff --git a/device-types/net/description.tex 
> b/device-types/net/description.tex
> index e71e33b..0d9f4b9 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -83,6 +83,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network 
> Device / Feature bits
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>  channel.
>
> +\item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue notification 
> coalescing.
> +
>  \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>
>  \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -139,6 +141,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device 
> Types / Network Device
>  \item[VIRTIO_NET_F_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or 
> VIRTIO_NET_F_HOST_TSO6.
>  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> +\item[VIRTIO_NET_F_VQ_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
>  \end{description}
>
>  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / 
> Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -1508,6 +1511,14 @@ \subsubsection{Control Virtqueue}\label{sec:Device 
> Types / Network Device / Devi
>  If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
>  send control commands for dynamically changing the coalescing parameters.
>
> +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated:
> +\begin{itemize}
> +\item a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command to set 
> coalescing parameters of a given
> +  enabled transmit/receive virtqueue.
> +\item a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command to a 
> device, and the device responds with
> +  coalescing parameters of a given enabled transmit/receive virtqueue.
> +\end{itemize}
> +
>  \begin{note}
>  The behavior of the device in response to these commands is best-effort:
>  the device may generate notifications more or less