Re: [PATCH 4/4] first conntrack ID must be 1 not 2

[Patrick McHardy <kaber@trash.net>]

Amin Azez wrote:
> Patrick McHardy wrote:
> 
>>> It's no longer the entry of the next conntrack but the entry of the last
>>> conntrack by the time it gets deleted then...? Or have I misunderstood
>>> the way in which the loop would occur?
>>>   
>>
>>
>> Yes, the loop is interrupted when the skb is full and continued when
>> userspace issues the next recvmsg(). The reference is kept to find
>> the point where to continue.
>>  
>>
> Did you mean "yes, you misunderstood" or "yes, good idea"?

Yes, you misunderstood :)

> I thought that what would theoretically cause the problem if the
> next-conntrack is deleted was the fact that conntrack-put is called
> before ctnetlink_fill_info.

Its only a marker, once we have found it in the list we don't
need that reference anymore.

> The problem may not be entirely theoretical if the conntrack is being
> deleted by a userspace conntrack client in response to what it reads
> when enumerating conntracks. (The conntrack may spring back into
> existance as an resurrected established connection - and also may
> therefore be likely in the case of a full contrack table where
> connections flip-flop their resurrected conntrack entry,
> Rather than issue warnings to conntrack developers to avoid certain
> kinds of design, it would be better to avoid this.

The marker points to the next entry that beed dumped yet, so
enumerate-and-delete won't affect it. It is also not the
ID of the connection but the pointer to the conntrack entry
that is used, so resurrections don't affect it in any way.

The problem is in my opinion purely theoretical so far:

- the bucket beeing dumped must contain enough entries to exceed
  the size of a single netlink message (very unlikely)

- when continuing to dump, the entry we wanted to dump next
  must be gone - and there must still be enough entries in the
  bucket to exceed the skb.

For an endless loop we must have a steady refill of the bucket
with new conntrack entries, otherwise at some point it won't
exceed the skb anymore and we can move on to the next bucket.
The jenkins hash should prevent this situation.