Re: [idpf] Gentle reminder for IDPF Spec review version 0.9 posted in Early Feb

["Michael S. Tsirkin" <mst@redhat.com>]

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