Infrastructure Data-Plane Function

Hi All,    IF you haven’t had a chance to review please do and send any feedback to the group. I plan on send the next version for review by End of April/Early May. Here are the additions that we have planned for the next review   Flow Steering offload flow and details completed. ADI offload flow and details QueueGroup usage and flow details. PTP offload flow and details. Any review fixes from the TC members received in next 2 weeks. There were a few that I got from Intel and Google folks that I will corporate as well.     Also hopefully you guys have noticed that there was a patch series from Intel sent out for a reference vendor driver that matches the 0.9 Spec. Currently its under /net/intel as it does not carry the Generic vendor/subvendor ID.   @Orr, Michael and @Michael S. Tsirkin please advice on the Vendor/subvendor ID that we should be suing for the common driver and also when should be a good milestone to promote the driver to be a generic driver.   Also please vote the version for next Spec.   Thanks Anjali

On Mon, Apr 10, 2023 at 05:27:01PM +0000, Singhai, Anjali wrote: > Hi All, > > IF you havenât had a chance to review please do and send any feedback to the > group. It's not currently easy to provide feedback: all we have is a PDF. What would I write? Example comment: The specification includes multiple instances of C code. For example: On page 9/204, we see: struct virtchnl_version_info { u32 major; u32 minor; }; but later: struct virtchnl_msg { u8 dev_mbox[8]; /* HW/SW Device mailbox fields */ /* see enum virtchnl_ops; avoid confusion with desc->opcode */ __le32 v_opcode:28; __le32 v_dtype:4; /* virtual channel descriptor Format */ /* see enum virtchnl_status_code; ditto for desc->retval */ __le32 v_retval; OASIS IDPF Technical Committee WIP â DRAFT Version 0.9 10/204 __le32 reserved; __le16 sw_cookie; __le16 v_flags; /* used by CP when sending messages to PF/VF */ struct v_dma_mem *payload; }; WRT this: - it is unclear what is struct v_dma_mem - copyright status of this code is unclear. I propose including a C header with all provided code together with the specification, and a permissive license such as the BSD license, or even placing it in the public domain, or finding some other way to document that everyone can copy this code. - why are some fields u32 but others __le32? On page 11, a structure is defined using a table. Why are some structres defined using a table with Bytes.Bits notation while others using the C notation? See how annoying it is? It is even harder to apply these comments: editors have to look up the relevant text then locate it in their editable copy. The lines are not even numbered so there's always a chance of ambiguity. I'd like to propose switching to some editable format that we can maintain in github. Applying patches then becomes a snap using standard git tools. Virtio has been using latex with some success. I wrote most of the scripts generating various versions of PDFs from latex used there so Red Hat holds copyright, and can donate them to IDPF if desired. Other ideas? > I plan on send the next version for review by End of April/Early May. > Here are the additions that we have planned for the next review > > > > 1. Flow Steering offload flow and details completed. > 2. ADI offload flow and details > 3. QueueGroup usage and flow details. > 4. PTP offload flow and details. > 5. Any review fixes from the TC members received in next 2 weeks. There were a > few that I got from Intel and Google folks that I will corporate as well. > Where were these posted? Comments from non-TC members must be posted on idpf-comment, to help make sure they are made under the terms of the feedbak license. Please do not accept comments in private mail. > > > > Also hopefully you guys have noticed that there was a patch series from Intel > sent out for a reference vendor driver that matches the 0.9 Spec. Currently its > under /net/intel as it does not carry the Generic vendor/subvendor ID. > That one has a GPL-only header with all these messages. This muddies the waters even more. > > @Orr, Michael and @Michael S. Tsirkin please advice on the Vendor/subvendor ID > that we should be suing for the common driver and also when should be a good > milestone to promote the driver to be a generic driver. > > > > Also please vote the version for next Spec. > > > > Thanks > > Anjali Hmm I am not sure what would you like to vote on? -- MST

Thanks Michael for the review. Lets put this on github for sure. About using latex for editing, let me set up sometime this coming week with you to learn what you have been doing with virtio spec. Thanks Anjali
Original Message----- From: Michael S. Tsirkin <mst@redhat.com> Sent: Thursday, May 11, 2023 11:37 PM To: Singhai, Anjali <anjali.singhai@intel.com> Cc: idpf@lists.oasis-open.org; Orr, Michael <michael.orr@intel.com> Subject: Re: [idpf] Gentle reminder for IDPF Spec review version 0.9 posted in Early Feb On Mon, Apr 10, 2023 at 05:27:01PM +0000, Singhai, Anjali wrote: > Hi All, > > IF you havenât had a chance to review please do and send any > feedback to the group. It's not currently easy to provide feedback: all we have is a PDF. What would I write? Example comment: The specification includes multiple instances of C code. For example: On page 9/204, we see: struct virtchnl_version_info { u32 major; u32 minor; }; but later: struct virtchnl_msg { u8 dev_mbox[8]; /* HW/SW Device mailbox fields */ /* see enum virtchnl_ops; avoid confusion with desc->opcode */ __le32 v_opcode:28; __le32 v_dtype:4; /* virtual channel descriptor Format */ /* see enum virtchnl_status_code; ditto for desc->retval */ __le32 v_retval; OASIS IDPF Technical Committee WIP â DRAFT Version 0.9 10/204 __le32 reserved; __le16 sw_cookie; __le16 v_flags; /* used by CP when sending messages to PF/VF */ struct v_dma_mem *payload; }; WRT this: - it is unclear what is struct v_dma_mem - copyright status of this code is unclear. I propose including a C header with all provided code together with the specification, and a permissive license such as the BSD license, or even placing it in the public domain, or finding some other way to document that everyone can copy this code. - why are some fields u32 but others __le32? On page 11, a structure is defined using a table. Why are some structres defined using a table with Bytes.Bits notation while others using the C notation? See how annoying it is? It is even harder to apply these comments: editors have to look up the relevant text then locate it in their editable copy. The lines are not even numbered so there's always a chance of ambiguity. I'd like to propose switching to some editable format that we can maintain in github. Applying patches then becomes a snap using standard git tools. Virtio has been using latex with some success. I wrote most of the scripts generating various versions of PDFs from latex used there so Red Hat holds copyright, and can donate them to IDPF if desired. Other ideas? > I plan on send the next version for review by End of April/Early May. > Here are the additions that we have planned for the next review > > > > 1. Flow Steering offload flow and details completed. > 2. ADI offload flow and details > 3. QueueGroup usage and flow details. > 4. PTP offload flow and details. > 5. Any review fixes from the TC members received in next 2 weeks. There were a > few that I got from Intel and Google folks that I will corporate as well. > Where were these posted? Comments from non-TC members must be posted on idpf-comment, to help make sure they are made under the terms of the feedbak license. Please do not accept comments in private mail. > > > > Also hopefully you guys have noticed that there was a patch series > from Intel sent out for a reference vendor driver that matches the 0.9 > Spec. Currently its under /net/intel as it does not carry the Generic vendor/subvendor ID. > That one has a GPL-only header with all these messages. This muddies the waters even more. > > @Orr, Michael and @Michael S. Tsirkin please advice on the > Vendor/subvendor ID that we should be suing for the common driver and > also when should be a good milestone to promote the driver to be a generic driver. > > > > Also please vote the version for next Spec. > > > > Thanks > > Anjali Hmm I am not sure what would you like to vote on? -- MST

You can find it all here: https://github.com/oasis-tcs/virtio-spec makediffall.sh gets you a redlined version using latexdiff while makeall.sh gets you a clean version changelog for each release has to be written manually, but there is gitlog.pl which helps one get started. On Fri, May 12, 2023 at 03:44:11PM +0000, Singhai, Anjali wrote: > Thanks Michael for the review. Lets put this on github for sure. About using latex for editing, let me set up sometime this coming week with you to learn what you have been doing with virtio spec. > > Thanks > Anjali > >
Original Message----- > From: Michael S. Tsirkin <mst@redhat.com> > Sent: Thursday, May 11, 2023 11:37 PM > To: Singhai, Anjali <anjali.singhai@intel.com> > Cc: idpf@lists.oasis-open.org; Orr, Michael <michael.orr@intel.com> > Subject: Re: [idpf] Gentle reminder for IDPF Spec review version 0.9 posted in Early Feb > > > On Mon, Apr 10, 2023 at 05:27:01PM +0000, Singhai, Anjali wrote: > > Hi All, > > > > IF you havenât had a chance to review please do and send any > > feedback to the group. > > It's not currently easy to provide feedback: all we have is a PDF. > What would I write? > Example comment: > > The specification includes multiple instances of C code. > For example: > On page 9/204, we see: > struct virtchnl_version_info { > u32 major; > u32 minor; > }; > > but later: > > struct virtchnl_msg { > u8 dev_mbox[8]; /* HW/SW Device mailbox fields */ > /* see enum virtchnl_ops; avoid confusion with desc->opcode */ > __le32 v_opcode:28; > __le32 v_dtype:4; /* virtual channel descriptor Format */ > /* see enum virtchnl_status_code; ditto for desc->retval */ > __le32 v_retval; > OASIS IDPF Technical Committee > WIP â DRAFT Version 0.9 10/204 > __le32 reserved; > __le16 sw_cookie; > __le16 v_flags; > /* used by CP when sending messages to PF/VF */ > struct v_dma_mem *payload; > }; > > > WRT this: > > - it is unclear what is struct v_dma_mem > > - copyright status of this code is unclear. > I propose including a C header with all provided > code together with the specification, and > a permissive license such as the BSD license, > or even placing it in the public domain, > or finding some other way to document that everyone can > copy this code. > > > - why are some fields u32 but others __le32? > > > On page 11, a structure is defined using a table. > Why are some structres defined using a table with Bytes.Bits notation while others using the C notation? > > > See how annoying it is? It is even harder to apply these comments: > editors have to look up the relevant text then locate it in their editable copy. The lines are not even numbered so there's always a chance of ambiguity. > > > > I'd like to propose switching to some editable format that we can maintain in github. Applying patches then becomes a snap using standard git tools. Virtio has been using latex with some success. I wrote most of the scripts generating various versions of PDFs from latex used there so Red Hat holds copyright, and can donate them to IDPF if desired. > Other ideas? > > > > > > > I plan on send the next version for review by End of April/Early May. > > Here are the additions that we have planned for the next review > > > > > > > > 1. Flow Steering offload flow and details completed. > > 2. ADI offload flow and details > > 3. QueueGroup usage and flow details. > > 4. PTP offload flow and details. > > 5. Any review fixes from the TC members received in next 2 weeks. There were a > > few that I got from Intel and Google folks that I will corporate as well. > > > > Where were these posted? Comments from non-TC members must be posted on idpf-comment, to help make sure they are made under the terms of the feedbak license. Please do not accept comments in private mail. > > > > > > > > > > Also hopefully you guys have noticed that there was a patch series > > from Intel sent out for a reference vendor driver that matches the 0.9 > > Spec. Currently its under /net/intel as it does not carry the Generic vendor/subvendor ID. > > > > That one has a GPL-only header with all these messages. This muddies the waters even more. > > > > > > @Orr, Michael and @Michael S. Tsirkin please advice on the > > Vendor/subvendor ID that we should be suing for the common driver and > > also when should be a good milestone to promote the driver to be a generic driver. > > > > > > > > Also please vote the version for next Spec. > > > > > > > > Thanks > > > > Anjali > > Hmm I am not sure what would you like to vote on? > > > -- > MST >