On 2/27/26 6:42 AM, Randy Dunlap wrote:
> 
> 
> On 2/26/26 2:57 PM, Ilya Maximets wrote:
>> On 2/26/26 11:10 PM, Randy Dunlap wrote:
>>> Fix some kernel-doc warnings in openvswitch.h:
>>>
>>> Mark enum placeholders that are not used as "private" so that kernel-doc
>>> comments are not needed for them.
>>>
>>> Correct names for 2 enum values:
>>> Warning: include/uapi/linux/openvswitch.h:300 Excess enum value
>>>  '@OVS_VPORT_UPCALL_SUCCESS' description in 'ovs_vport_upcall_attr'
>>> Warning: include/uapi/linux/openvswitch.h:300 Excess enum value
>>>  '@OVS_VPORT_UPCALL_FAIL' description in 'ovs_vport_upcall_attr'
>>>
>>> Warning: include/uapi/linux/openvswitch.h:638 This comment starts with
>>>  '/**', but isn't a kernel-doc comment.
>>>  * Omit attributes for notifications.
>>>
>>> Signed-off-by: Randy Dunlap <[email protected]>
>>> ---
>>
>> Hi, Randy.  Thanks for the fixes!
>>
>> If the private/public is the style to follow, it sounds good to me,
>> though there seem to be an inconsistency in how the kernel-only
>> attributes are marked.  I see the OVS_SAMPLE_ATTR_ARG is in the
>> private section, but OVS_CHECK_PKT_LEN_ATTR_ARG and the
>> OVS_ACTION_ATTR_SET_TO_MASKED are public.  They should either all
>> be public or all private, as they are used for very similar purposes,
>> which is storing extra information about the action to make it easier
>> for the kernel to work with them.  These attributes should never be
>> used in the communication with userspace.  So, maybe private?
> 
> I documented them and made them all public:.

OK.

> 
>>
>> __OVS_CHECK_PKT_LEN_ATTR_MAX is also public for some reason.  I guess,
>> it is because while it has kernel-doc-style comment, it doesn't start
>> with /**, so it's not treated like one.  Maybe we should fix that here
>> just to be consistent?
> 
> I didn't follow the "it doesn't start with /**", but I made it private:.

The point was that the comment for the 'enum ovs_check_pkt_len_attr'
starts with the regular '/*' and not the kernel-doc standard '/**'.
So I assumed that's the reason why it was missed.

Looking more closely, the same applies also to the
'struct ovs_action_push_eth', that documents a filed it doesn't have...

> 
>> note: Some enums and structures do not have kernel-doc comments, so
>> I guess that's the reason to not mark them.  I suppose it's fine,
>> since adding extra docs is not the purpose of this patch.
> 
> I ended up adding comments for several enums, but you might want to correct
> a few of them.

Sure.

> 
>> nit: the subject prefix is a little strange.
> 
> I hope that I fixed that.
> I'll send v2 after 24++ hours.
> 
> thanks.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to