Re: [PATCH 4/4] netfilter: xtables: merge registration structure to NFPROTO_UNSPEC

[Patrick McHardy <kaber@trash.net>]

Jan Engelhardt wrote:
> On Wednesday 2010-03-31 11:08, Patrick McHardy wrote:
>   
>> Jan Engelhardt wrote:
>>     
>>> On Wednesday 2010-03-31 11:01, Patrick McHardy wrote:
>>>       
>>>> Jan Engelhardt wrote:
>>>>         
>>>>>>> This will work because x_tables scans for NFPROTO_UNSPEC,
>>>>>>> and arp/ebtables just using x_tables :-)
>>>>>>>               
>>>>>> I'm not sure I'm parsing this correctly. Both will find the match,
>>>>>> however the nf_ct_l3proto_try_module_get() call will fail
>>>>>>             
>>>>> It won't fail - it is using par->family, not par->match->family.
>>>>>           
>>>> That's broken then.
>>>>         
>>> How so?
>>>       
>> Because arptables and ebtables shouldn't be able to use this module
>> directly. Even less so after a patch stating "merge registration
>> structure".
>>     
>
> arp/ebtables _couldn't_ even use this module. The simple showstopper:
> arp/ebtables simply don't have a corresponding userspace portion for
> it.

That's a really bad argument.

>  Indeed nf_ct_l3proto_try_module_get(NFPROTO_BRIDGE) does not make
> much sense, but, in all honesty, xt_state *is* testing for a
> protocol-independent feature, so NFPROTO_UNSPEC is justified IMO.
>   
Agreed.
> Also, NFPROTO_BRIDGE is special anyway - it does not refer to an L3
> protocol actually, but to L2 - so, well, it's kinda moot to muse
> about the possibility of calling nf_ct_get(NFPROTO_BRIDGE).

I assume you mean nf_ct_l3proto_try_module_get(). Just as I was saying,
it *will* fail for NFPROTO_BRIDGE/ARP, so everything should be fine. You
disputed this however.

>  If you
> _really_ wanted to support state matching at the ARP/EB level, you
> would anyhow have to add a separate ->check function that loads all
> possible L3 trackers. Which is not a big problem per se
> (see patch - no touching of NFPROTO_UNSPEC was needed).
>   

That doesn't really work since bridge netfilter is (partially) invoked
before conntrack.