libnl: Unmatched NL_ACT_DEL and NL_ACT_CHANGE

libnl: Unmatched NL_ACT_DEL and NL_ACT_CHANGE

[Volker Poplawski]

Hi all.

Could s.o. please  have a look at my (short) code at
http://pastie.org/534637

(maybe compile it with g++ -Wall test.c -o test -I ... -L ... -lnl -lnl-genl -
lnl-nf -lnl-route)

What the code does is to listen to changes in the ct-table using libnl. It 
keeps score of reported ctId in a lookup table.

Problem is: I'm getting a lot of NL_ACT_CHANGE & NL_ACT_DEL without having 
seen a matching NL_ACT_NEW. (Also there seems to be no initial cache fill)

kernel 2.6.27 (opensuse 11.1), libnl 2.0 (git master)

Greetings
....Volker

Re: libnl: Unmatched NL_ACT_DEL and NL_ACT_CHANGE

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

Volker Poplawski wrote:
> Hi all.
> 
> Could s.o. please  have a look at my (short) code at
> http://pastie.org/534637
> 
> (maybe compile it with g++ -Wall test.c -o test -I ... -L ... -lnl -lnl-genl -
> lnl-nf -lnl-route)
> 
> What the code does is to listen to changes in the ct-table using libnl. It 
> keeps score of reported ctId in a lookup table.
> 
> Problem is: I'm getting a lot of NL_ACT_CHANGE & NL_ACT_DEL without having 
> seen a matching NL_ACT_NEW. (Also there seems to be no initial cache fill)
> 
> kernel 2.6.27 (opensuse 11.1), libnl 2.0 (git master)

I think I know whats happening - the ct objects don't define the
attribute(s) distinguishing different entries, so cache_include()
doesn't recognize them as new.

Does this patch make any difference?

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 347 bytes --]

diff --git a/lib/netfilter/ct_obj.c b/lib/netfilter/ct_obj.c
index ae14c0d..48a7abe 100644
--- a/lib/netfilter/ct_obj.c
+++ b/lib/netfilter/ct_obj.c
@@ -779,6 +779,7 @@ struct nl_object_ops ct_obj_ops = {
 	    [NL_DUMP_STATS]	= ct_dump_stats,
 	},
 	.oo_compare		= ct_compare,
+	.oo_id_attrs		= CT_ATTR_ID,
 	.oo_attrs2str		= ct_attrs2str,
 };
 

Re: libnl: Unmatched NL_ACT_DEL and NL_ACT_CHANGE

[Volker Poplawski]

On Monday 06 July 2009 13:29:40 you wrote:
> Volker Poplawski wrote:
> > Hi all.
> >
> > Could s.o. please  have a look at my (short) code at
> > http://pastie.org/534637
> >
> > (maybe compile it with g++ -Wall test.c -o test -I ... -L ... -lnl
> > -lnl-genl - lnl-nf -lnl-route)
> >
> > What the code does is to listen to changes in the ct-table using libnl.
> > It keeps score of reported ctId in a lookup table.
> >
> > Problem is: I'm getting a lot of NL_ACT_CHANGE & NL_ACT_DEL without
> > having seen a matching NL_ACT_NEW. (Also there seems to be no initial
> > cache fill)
> >
> > kernel 2.6.27 (opensuse 11.1), libnl 2.0 (git master)
>
> I think I know whats happening - the ct objects don't define the
> attribute(s) distinguishing different entries, so cache_include()
> doesn't recognize them as new.
>
> Does this patch make any difference?

Yes it does,  NL_ACT_DEL , _CHANGE and _DEL are now matching -- for ct-entries 
created after i made my call to nl_cache_mngr_add( ... "netfilter/ct"... )

However, i still don't get a NL_ACT_DEL on already existing connections 
(CHANGE and DEL though)

Re: libnl: Unmatched NL_ACT_DEL and NL_ACT_CHANGE

[Philip Craig]

Volker Poplawski wrote:
> On Monday 06 July 2009 13:29:40 you wrote:
>> Volker Poplawski wrote:
>>> Hi all.
>>>
>>> Could s.o. please  have a look at my (short) code at
>>> http://pastie.org/534637
>>>
>>> (maybe compile it with g++ -Wall test.c -o test -I ... -L ... -lnl
>>> -lnl-genl - lnl-nf -lnl-route)
>>>
>>> What the code does is to listen to changes in the ct-table using libnl.
>>> It keeps score of reported ctId in a lookup table.
>>>
>>> Problem is: I'm getting a lot of NL_ACT_CHANGE & NL_ACT_DEL without
>>> having seen a matching NL_ACT_NEW. (Also there seems to be no initial
>>> cache fill)
>>>
>>> kernel 2.6.27 (opensuse 11.1), libnl 2.0 (git master)
>> I think I know whats happening - the ct objects don't define the
>> attribute(s) distinguishing different entries, so cache_include()
>> doesn't recognize them as new.
>>
>> Does this patch make any difference?

Yes that improves it.  We probably want to change libnl so that we
can specify a set of optional attributes to compare, so that
nl_object_identical does something like this:

	if ((a->ce_mask & req_attrs) != req_attrs ||
	    (b->ce_mask & req_attrs) != req_attrs)
		return 0;

	if ((a->ce_mask & opt_attrs) != (b->ce_mask & opt_attrs))
		return 0;
	...
	return !(ops->oo_compare(a, b, (req_attrs | a->ce_mask & opt_attrs), 0));


This would let it work on older kernels that don't include the id too.

> Yes it does,  NL_ACT_DEL , _CHANGE and _DEL are now matching -- for ct-entries 
> created after i made my call to nl_cache_mngr_add( ... "netfilter/ct"... )
> 
> However, i still don't get a NL_ACT_DEL on already existing connections 
> (CHANGE and DEL though)

I assume you meant you don't get NL_ACT_NEW events for existing
connections.  That's just how libnl works in general.  You can use
nl_cache_get_first/nl_cache_get_next to populate your hashtable
before you start polling.

Re: libnl: Unmatched NL_ACT_DEL and NL_ACT_CHANGE

[Volker Poplawski]

On Tuesday 07 July 2009 07:29:42 you wrote:
> Volker Poplawski wrote:
> > On Monday 06 July 2009 13:29:40 you wrote:
> >> Volker Poplawski wrote:
> >>> Hi all.
> >>>
> >>> Could s.o. please  have a look at my (short) code at
> >>> http://pastie.org/534637
> >>>
> >>> (maybe compile it with g++ -Wall test.c -o test -I ... -L ... -lnl
> >>> -lnl-genl - lnl-nf -lnl-route)
> >>>
> >>> What the code does is to listen to changes in the ct-table using libnl.
> >>> It keeps score of reported ctId in a lookup table.
> >>>
> >>> Problem is: I'm getting a lot of NL_ACT_CHANGE & NL_ACT_DEL without
> >>> having seen a matching NL_ACT_NEW. (Also there seems to be no initial
> >>> cache fill)
> >>>
> >>> kernel 2.6.27 (opensuse 11.1), libnl 2.0 (git master)
> >>
> >> I think I know whats happening - the ct objects don't define the
> >> attribute(s) distinguishing different entries, so cache_include()
> >> doesn't recognize them as new.
> >>
> >> Does this patch make any difference?
>
> Yes that improves it.  We probably want to change libnl so that we
> can specify a set of optional attributes to compare, so that
> nl_object_identical does something like this:
>
> 	if ((a->ce_mask & req_attrs) != req_attrs ||
> 	    (b->ce_mask & req_attrs) != req_attrs)
> 		return 0;
>
> 	if ((a->ce_mask & opt_attrs) != (b->ce_mask & opt_attrs))
> 		return 0;
> 	...
> 	return !(ops->oo_compare(a, b, (req_attrs | a->ce_mask & opt_attrs), 0));
>
>
> This would let it work on older kernels that don't include the id too.
>
> > Yes it does,  NL_ACT_DEL , _CHANGE and _DEL are now matching -- for
> > ct-entries created after i made my call to nl_cache_mngr_add( ...
> > "netfilter/ct"... )
> >
> > However, i still don't get a NL_ACT_DEL on already existing connections
> > (CHANGE and DEL though)
>
> I assume you meant you don't get NL_ACT_NEW events for existing
> connections.  That's just how libnl works in general.  You can use
> nl_cache_get_first/nl_cache_get_next to populate your hashtable
> before you start polling.
Uups, typo.

nl_cache_get_first/nl_cache_get_next did the trick.

Thanx

Re: libnl: Unmatched NL_ACT_DEL and NL_ACT_CHANGE

[Patrick McHardy]

Philip Craig wrote:
> Volker Poplawski wrote:
>> On Monday 06 July 2009 13:29:40 you wrote:
>>> Volker Poplawski wrote:
>>>> Hi all.
>>>>
>>>> Could s.o. please  have a look at my (short) code at
>>>> http://pastie.org/534637
>>>>
>>>> (maybe compile it with g++ -Wall test.c -o test -I ... -L ... -lnl
>>>> -lnl-genl - lnl-nf -lnl-route)
>>>>
>>>> What the code does is to listen to changes in the ct-table using libnl.
>>>> It keeps score of reported ctId in a lookup table.
>>>>
>>>> Problem is: I'm getting a lot of NL_ACT_CHANGE & NL_ACT_DEL without
>>>> having seen a matching NL_ACT_NEW. (Also there seems to be no initial
>>>> cache fill)
>>>>
>>>> kernel 2.6.27 (opensuse 11.1), libnl 2.0 (git master)
>>> I think I know whats happening - the ct objects don't define the
>>> attribute(s) distinguishing different entries, so cache_include()
>>> doesn't recognize them as new.
>>>
>>> Does this patch make any difference?
> 
> Yes that improves it.  We probably want to change libnl so that we
> can specify a set of optional attributes to compare, so that
> nl_object_identical does something like this:
> 
> 	if ((a->ce_mask & req_attrs) != req_attrs ||
> 	    (b->ce_mask & req_attrs) != req_attrs)
> 		return 0;
> 
> 	if ((a->ce_mask & opt_attrs) != (b->ce_mask & opt_attrs))
> 		return 0;
> 	...
> 	return !(ops->oo_compare(a, b, (req_attrs | a->ce_mask & opt_attrs), 0));
> 
> 
> This would let it work on older kernels that don't include the id too.

Good point, we didn't include the ID in event messages in older
kernels. We should also compare the tuples anyways, which should
work on both old and new kernels since the ID won't be available
for both compared objects and thus there's no difference if I read
the code correctly.