Use the correct sequence sequence of check and action.

Only improve comments.

In D53564#1223644, @glebius wrote:

In D53564#1223423, @tuexen wrote:

Maybe we want separate counters for:

• syncache_repsonse() failed
• syncache entries are dropped.

Yes, this is the way to go! Can you please commit this revision as comment change only and create a new one that would make two counters and improve descriptions in netstat?

In D53564#1223412, @glebius wrote:

I see your logic, but then we need to reformat netstat, too. And that probably shouldn't get MFC-ed to stable/15.

2 syncache entries added
        0 retransmitted
        0 dupsyn
        0 dropped
        2 completed
        0 bucket overflow
        0 cache overflow
        0 reset
        0 stale
        0 aborted
        0 badack
        0 unreach
        0 zone failures
2 cookies sent
        0 cookies received
        0 spurious cookies rejected
        0 failed cookies rejected

We don't want dropped to grow as challenge ack sending fails.

In D53564#1223411, @glebius wrote:

In the third case we also drop. In the first two cases we called syncache_drop() that would unlink and free the entry. In the third case we have already allocated an entry and we were about to link it in, but we resulted in freeing it.

The third case is only a drop, if we have allocated an entry. If we are using syncookies, we also increment the counter if the syncache_response() call failed.

The suggested fourth case is in its nature different to the first three.

This depends on the semantic of the counter:

• if the semantic is "reply is dropped", it is the same and it should be incremented.
• if the semantic is "syncache entry is dropped", it is not the same and the third pattern should be changed to increment the counter only if we are actually calling syncache_free().

In D53564#1223369, @glebius wrote:

tcp_var.h is not source of truth here! This file and also netstat(1) are. In this file the counter only is updated when we indeed drop. In netstat counter is reported to user as "dropped". This change should be a comment only change.

Use the correct variable: inp->inp_flags2.

This was committed in f3bba8cd62f2.

In D53498#1221585, @tuexen wrote:

In D53498#1221542, @markj wrote:

This code should really use %b, as documented in printf(9).

I agree. But we have this kind of code in more than one place. I can take a look.

Have a look at D53507 and D53510 which does this in the networking related files.

remove one flag (INP_ORIGDSTADDR), which is actually a flag used in inp_flags2 and not in inp_flags.

Don't we want to print inp_flags2 as well? BTW, it seems very easy to use the wrong flag field. It should at least be called INP2_ORIGDSTADDR or something like that.

Yes, we want to print inp_flags2, too. This will be a separate change. And I agree with your proposed name change. There is something like INP_2PCP_SET, but I would prefer something like INP2_PCP_SET or INP_2_PCP_SET. Any preference?

Use tabs consistently.

Whitespace change.

In D53498#1221542, @markj wrote:

This code should really use %b, as documented in printf(9).

I agree. But we have this kind of code in more than one place. I can take a look.

remove one flag (INP_ORIGDSTADDR), which is actually a flag used in inp_flags2 and not in inp_flags.

Don't we want to print inp_flags2 as well? BTW, it seems very easy to use the wrong flag field. It should at least be called INP2_ORIGDSTADDR or something like that.

Yes, we want to print inp_flags2, too. This will be a separate change. And I agree with your proposed name change. There is something like INP_2PCP_SET, but I would prefer something like INP2_PCP_SET or INP_2_PCP_SET. Any preference?

Address Mark's comment.