Victor Kaplansky
2015-Aug-19 11:31 UTC
[PATCH v2 2/2] virtio-net: add default_mtu configuration field
On Mon, Aug 17, 2015 at 11:07:15AM +0800, Jason Wang wrote:> > > On 08/16/2015 09:42 PM, Victor Kaplansky wrote: > > @@ -3128,6 +3134,7 @@ struct virtio_net_config { > > u8 mac[6]; > > le16 status; > > le16 max_virtqueue_pairs; > > + le16 default_mtu; > > Looks like "mtu" is ok, consider we use "mac" instead of "default_mac".Good point. I'll change the name in the next version of the patch.> > > }; > > \end{lstlisting} > > > > @@ -3158,6 +3165,15 @@ by the driver after negotiation. > > \field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is > > set and can be read by the driver. > > > > +\item [\field{default_mtu}] is a hint to the driver set by the > > + device. It is valid during feature negotiation only if > > + VIRTIO_NET_F_DEFAULT_MTU is offered and holds the initial value > > + of MTU to be used by the driver. If VIRTIO_NET_F_DEFAULT_MTU is > > + negotiated, the driver uses the \field{default_mtu} as an initial > > + value, and also reports MTU changes to the device by writes to > > + \field{default_mtu}. Such reporting can be used for debugging, > > + or it can be used for tunning MTU along the network. > > + > > I vaguely remember that config is read only in some arch or transport > and that's why we introduce another vq cmd to confirm the announcement. > Probably we should do same for this?If so, we need to add one more feature bit to confirm the ability of the driver to report MTU, or we can weaken the requirement in conformance statement and write "the driver may report the MTU". What do you say? -- Victor
Jason Wang
2015-Aug-20 02:48 UTC
[PATCH v2 2/2] virtio-net: add default_mtu configuration field
On 08/19/2015 07:31 PM, Victor Kaplansky wrote:> On Mon, Aug 17, 2015 at 11:07:15AM +0800, Jason Wang wrote: >> >> On 08/16/2015 09:42 PM, Victor Kaplansky wrote: >>> @@ -3128,6 +3134,7 @@ struct virtio_net_config { >>> u8 mac[6]; >>> le16 status; >>> le16 max_virtqueue_pairs; >>> + le16 default_mtu; >> Looks like "mtu" is ok, consider we use "mac" instead of "default_mac". > Good point. I'll change the name in the next version of the patch. > >>> }; >>> \end{lstlisting} >>> >>> @@ -3158,6 +3165,15 @@ by the driver after negotiation. >>> \field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is >>> set and can be read by the driver. >>> >>> +\item [\field{default_mtu}] is a hint to the driver set by the >>> + device. It is valid during feature negotiation only if >>> + VIRTIO_NET_F_DEFAULT_MTU is offered and holds the initial value >>> + of MTU to be used by the driver. If VIRTIO_NET_F_DEFAULT_MTU is >>> + negotiated, the driver uses the \field{default_mtu} as an initial >>> + value, and also reports MTU changes to the device by writes to >>> + \field{default_mtu}. Such reporting can be used for debugging, >>> + or it can be used for tunning MTU along the network. >>> + >> I vaguely remember that config is read only in some arch or transport >> and that's why we introduce another vq cmd to confirm the announcement. >> Probably we should do same for this? > If so, we need to add one more feature bit to confirm the ability > of the driver to report MTU, or we can weaken the requirement in > conformance statement and write "the driver may report the MTU". > What do you say? > > -- VictorEither looks good for me. (Need confirm the read only issue with the editors of other transport or arch) Thanks
Apparently Analagous Threads
- [PATCH v2 2/2] virtio-net: add default_mtu configuration field
- [PATCH v2 2/2] virtio-net: add default_mtu configuration field
- [PATCH v2 2/2] virtio-net: add default_mtu configuration field
- [PATCH v2 2/2] virtio-net: add default_mtu configuration field
- [PATCH v2 0/2] virtio-net: default_mtu - new conf. field