Re: [PATCH net] netlink: specs: fix the conntrack filter type

On 8/26/26 6:38 PM, Ilya Maximets wrote:
> On 8/26/26 7:20 PM, Asbjørn Sloth Tønnesen wrote:
>> On 8/25/26 3:58 PM, Ilya Maximets wrote:
>>> The CTA_FILTER doesn't contain nested tuple attributes, instead it
>>> contains bit masks that specify which tuple attributes to filter on.
>>> The values for filtering are taken from the top-level CTA_TUPLE_ORIG
>>> and CTA_TUPLE_REPLY.
>>
>> As identified by Sashiko-NIPA[1], then CTA_TUPLE_ORIG and CTA_TUPLE_REPLY
>> are both absent from the GET DUMP request attribute list.
>>
>> However this doesn't prevent ynl CLI from sending the attributes:
>>
>> $ sudo ./tools/net/ynl/pyynl/cli.py --family conntrack --dump get --json \
>>     '{"nfgen-family": 2, "filter": {"orig-flags": 1}}'
>> Netlink error: Invalid argument
>>
>> $ sudo ./tools/net/ynl/pyynl/cli.py --family conntrack --dump get --json \
>> '{"nfgen-family": 2, "filter": {"orig-flags": 1}, "tuple-orig": {"tuple-ip": {"ip-v4-src": "192.255.0.255", "ip-v4-dst":
>> "192.255.255.0"}}}'
>> []
>>
>> Otherwise, LGTM.
>>
>> [1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260825155832.3685714-1-i.maximets%40ovn.org
> 
> The reported issue is preexisting and a little orthogonal to the goal of
> this patch.  I.e. missing dump attributes vs wrong type of attributes that
> are specified.
> 
> I can add all the missing attributes: tuple-orig, tuple-reply, mark-mask
> and status-mask.  Either as a separate patch, or fold the change into this
> one, but rename it into something like:
>    "netlink: specs: fix conntrack filtering attributes and types"
> 
> WDYT?

I agree that it is a little orthogonal, but OTOH the patch doesn't really fix
anything meaningful when the YNL spec doesn't allow actually making the request.
The commit message directly refers to attributes not allowed in the YNL spec,
without declaring it as intentionally deferred (like the filter bitmasks).

I see mark-mask and status-mask as a fully orthogonal issue, as they don't
clash with the commit message.

I would just fold tuple-orig and tuple-reply into this patch, and leave
mark-mask and status-mask for another patch.  Alternatively make it a series.