Reduce copy-n-paste: macro for hash and tcp_hc_cmp().

As Mark suggested split tcp_hc_lookup() into lookup-for-read and
lookup-for-write versions, and then inline the latter into tcp_hc_update().

Use atomic loads when reading hostcache values, as suggested by Mark.

Address markj@'s comments.

Converted hash buckets to CK_SLIST.

What are the downsides of using label for that purpose? Does it have any extra functionality except giving a human readable label? If so, killing states by this label is doable. Trying to imagine a situation where we require first label for just human readable label and second label (schedule) for deletion. Why can't the former be used for deletion as well?

P.S. Sorry, always forgetting about Chelsio. :(

Thanks!

AFAIU, the goal is to distinguish states created by certain rule(s). And in this particular case to kill those states. However, states already reference rules, so internally we can always attribute them. The problem is that pf(4) doesn't have rule numbers and we thus can't specify a rule in pfctl. So this change introduces "rule labels" instead of rule numbers. Btw, why "schedule" and not just "label"? This all makes me think that in future you might want more functionality that would require pointing at particular rules from pfctl or other tool. So, may be this rule specifying functionality needs to be developed in a more generic way, and then provide a control to kill states by rule.

I think this change should be reviewed wider than just to close a problem with LACP. Why do we prioritize servicing interrupts that are triggered by remote agents higher than our own callout routines? Note that NICs, unlike disks, are source of "foreign" interrupts. Also, the nature of network is lossy, while callouts are supposed to be executed in time.

In D29576#663233, @ae wrote:

I think there is a small window after SCH_UNLOCK() and before syncache_insert() when another threads can do lookup, create entry and come to the same point when it ready to syncache_insert(). And there are no guaranties that they will not do that :)
I seen some strange cases, when packets were delayed for 3 seconds where this looks impossible. And this happened several times on different machines, so this highly depends on the scheduling :)
Another question how it is dangerous for us. I just say, that if we want to change INP_WLOCK to INP_RLOCK, we need to make syncache_lookup() again when we are ready to make syncache_insert() with SCH_LOCK() held. Or rework somehow locking in syncache to avoid this. Or we need to make sure, that syncache duplicates will not lead to worse things.

In D29576#663233, @ae wrote:

I think there is a small window after SCH_UNLOCK() and before syncache_insert() when another threads can do lookup, create entry and come to the same point when it ready to syncache_insert(). And there are no guaranties that they will not do that :)

In D29576#663187, @ae wrote:

Imagine, you have two or more identical SYNs, now you use INP_RLOCK(), this means they all can be handled in parallel "in the same time". This means, syncache_add() in some cases can determine that there are no corresponding entries yet - syncache_lookup() returns NULL, we still hold SCH_LOCK(), then we allocate syncache entry, populate all its fields and do SCH_UNLOCK(). Imagine, now the same things will be done by the another thread, and another. In the end we will have several the same entries added by the syncache_insert().
Is this impossible?

In D29576#663155, @ae wrote:

I doubt that it is enough to just do s/wlock/rlock/ in the syncache code. Usually we need to add the extra check that an entry was not already linked by another thread in such cases.

They originally were pointers and counter_u64_free(NULL) worked. That was regressed with fb886947d97375ea9906fa1396f83573b6d0674b I think

I would suggest to change pfsync_request_update() to unconditionally make pfsync_push() on the bucket, regardless of request type (bulk or a single state). This will allow for some batching and shorten stuff executed from the callout context. After this change the call to pfsync_sendout() from the pfsync_request_update() can be changed to 0 in first arguemnt.

IMHO, a good sysadmin should always specify mask. And good program should require mask. However, there was historical behavior to guess mask based on classes. Thus, my personal preference list is the following:

Stefan, thanks a lot of taking care of my fallout!

• Revert 97ec6eba653a07. There shouldn't be a dependency of 'tmp' on
• Add 'tmp' to the list of FILESYSTEMS dependencies. Some scripts that

This may have a slight improvement when compiling from a non-branch point - a detached state. Now