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

On 8/26/26 9:40 PM, Asbjørn Sloth Tønnesen wrote:
> 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.

Makes sense.  I'll make it a set:  First patch will be this one with the
tuple-orig and tuple-reply folded in.  The second will add missing
mark-mask and status-mask.

Best regards, Ilya Maximets.