This change seems to revert the commit 6134aabe38c8. Is there any behavior change on V_functions_inherit_listen_socket_stack == 0 after this change? Is the TCP function block from the listener changed dynamically once the default stack is changed?

ship after update

Looks good to me, after the comment update for struct cc_var.

In D46066#1051183, @tuexen wrote:

In D46066#1050919, @cc wrote:

Just saw https://reviews.freebsd.org/D46068. If this patch has not been committed, we can still further revise it, so that D46068 can be cleaner on function re-use.

I think the logic here can be split into two functions, such that the refined logic 1 on checking if we should send ACK can be re-used across multiple places.

logic 1:

bool
is_ack_unlimited(struct tcpcb *tp)
{
    /* focus on checking the epoch and
     * if should send ACK, return true; else return false
     */ 
}

logic 2:

void
tcp_send_challenge_ack(struct tcpcb *tp, struct tcphdr *th, struct mbuf *m)
{
    if (is_ack_unlimited(tp)) {.
       tcp_respond();
    ....
    }
}

I don't get the point. Eventually, all places should use tcp_send_challenge_ack(), I think. But that is multiple commits away.

Just saw https://reviews.freebsd.org/D46068. If this patch has not been committed, we can still further revise it, so that D46068 can be cleaner on function re-use.

code clean up

Thanks for the update. Looks better and just a small concern about TCP_MAXWIN for 65535.

After reading the paper and IETF slides, it looks good to me on top of the general view. Just some small concerns below.

Thanks for the update! Looks good to me.

Over all looks good to me. Thanks for the update.

I started to draft the results into wiki pages from testing this patch. Please let me know any metrics you would like to see.

Other than the comment wording like "rtwn supports the default set of net80211 supported ciphers (IEEE80211_CRYPTO_[WEP, TKIP, AES_CCM])", looks good to me.

Looks good to me. :)

Start to pick up slowly about where I left, and touch the water. :)

I see you made comments of "ieee80211_set_software_ciphers() is not needed" in if_rtwn.c. Do you mind to add it in the rest of the drivers files? Or is there any better/central place to put such comment or document it?

update based on bz@ comments

rebase before scrutiny check

Individual accept.

Just some minor issues.

If you have a plan to figure out TEXT_SET(crypto##_set, name##_modevent) and TEXT_SET(ratectl##_set, alg##_modevent), I have no problem with this patch.

Cautious individual accept.

I don't get it why this change made a difference in your test. Is it because of the bug I pointed in TEXT_SET ?

LGTM

The rest looks good to me.

In D44827#1023217, @adrian wrote:

In D44827#1023209, @bz wrote:

In D44827#1023208, @adrian wrote:

eg ath(4) would do something like:

How do you want to implement ath_settkipmic() "nicely" without exposing the field?

oops my bad. it'll be a flag for hardware encryption support! I fixed the code snippet.

Also do you have any plan to update the man page regarding the new field ic_sw_cryptocaps besides ic_cryptocaps ?

Also please update the Summary section in this review, i.e. the mentioned new field ic_wpa_cryptocaps is actually the ic_sw_cryptocaps in the code change.

LGTM

I am intending to give approval, but I am afraid of missing the new response to my review comment for the code comment update. So let's be patient. :)

In D43966#1023788, @bz wrote:

@cc Adding LOCK asserts is a very good idea; not all the functions need them if I remember correctly. Should I really add it to this "fix" or should I do a separate full pass on the linux_80211_macops.c file?

In D44463#1014016, @adrian wrote:

you negotiate it as an extension, and then it's used to switch STA keys without dropping frames.

During association with the AP, sw/hw keys (indexes) are set. And before tx, the hw key is selected by sw key's index returned from ieee80211_crypto_get_txkey(), same way as without LKPI_80211_HW_CRYPTO.

I am working the "multi-key" support these days on top of D43634(this patch) and D43648, but it turns out this patch is causing more trouble than it is trying to solve.
In fact, it caused this "Abort trap" from the console and then my VM is shutdown silently. The kernel is main with LKPI_80211_HW_CRYPTO on top of this patch.

code update according to comments

I think LKPI_80211_LHW_LOCK_ASSERT can help remind us of such LKPI_80211_LHW_LOCK requirement for lkpi_80211_mo_* functions.

The ktls draws my attention. Hope you add reviewers.

Shall correct the typo first before committing.

Please add PR: 277095 in the summary, as this patch also fixes that.

Good catch! Always keep an eye on the cubic_data->t_epoch refresh.

In D43968#1003515, @bz wrote:

In D43968#1003463, @cc wrote:

Initial test shows this error when I changed the channel number in AP. Looks like a reproduce of PR 277100 without LKPI_80211_HW_CRYPTO enabled, or a separate issue?

Yes, looks likely same/similar than in the PR.

You've seen the comments (1)-(4) above?

Initial test shows this error when I changed the channel number in AP. Looks like a reproduce of PR 277100 without LKPI_80211_HW_CRYPTO enabled, or a separate issue?

My initial test on the three patches of D43389, D43725, D43753 looks to be good. No more panics. And I need to figure out some issues in my testbed, so I give the approval first as I don't want my test to delay the schedule.

My initial test on the three patches of D43389, D43725, D43753 looks to be good. No more panics. And I need to figure out some issues in my testbed, so I give the approval first as I don't want my test to delay the schedule.

My initial test on the three patches of D43389, D43725, D43753 looks to be good. No more panics. And I need to figure out some issues in my testbed, so I give the approval first as I don't want my test to delay the schedule.

In D43770#999610, @bz wrote:

Can we please NOT do this. I explained it in my other review (D43658) twice by now.

This is (a) only changing some defines to change and not all the others for the same structure, (b) it's been in at least 3 major releases this way why still change it? It fits with the entire HT code. For VHT which is not used yet we still can do and do.

Also, if doing I said replace them with the names from LinuxKPI so we do not have two versions. You are just trading one name for another name and still leaving us with more compat names.

In D43725#999202, @cc wrote:

In D43725#999194, @cc wrote:

Given D43389 is the FIRST of a series, is this the SECOND one or is there any dependence? Please help clarify.

I think there is dependence, as I applied this patch only, restarted netif, and hit the panic:

--- trap 0x9, rip = 0xffffffff80cf8661, rsp = 0xfffffe00ab111d00, rbp = 0xfffffe00ab111d10 ---
node_free() at node_free+0x11/frame 0xfffffe00ab111d10
lkpi_sta_auth_to_scan() at lkpi_sta_auth_to_scan+0x27f/frame 0xfffffe00ab111d80
lkpi_iv_newstate() at lkpi_iv_newstate+0x253/frame 0xfffffe00ab111df0
ieee80211_newstate_cb() at ieee80211_newstate_cb+0x226/frame 0xfffffe00ab111e40
taskqueue_run_locked() at taskqueue_run_locked+0xab/frame 0xfffffe00ab111ec0
taskqueue_thread_loop() at taskqueue_thread_loop+0xd3/frame 0xfffffe00ab111ef0
fork_exit() at fork_exit+0x82/frame 0xfffffe00ab111f30
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe00ab111f30
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---
KDB: enter: panic
[ thread pid 0 tid 100192 ]
Stopped at      kdb_enter+0x32: movq    $0,0xe394d3(%rip)
db>

In D43658#999086, @emaste wrote:

I thought the ports tree contains user-space applications. Are you talking about some user-space drivers in the ports tree?

There are a number of kernel modules in the ports tree as well. drm-kmod is one well known example.

In D43725#999194, @cc wrote:

Given D43389 is the FIRST of a series, is this the SECOND one or is there any dependence? Please help clarify.

Given D43389 is the FIRST of a series, is this the SECOND one or is there any dependence? Please help clarify.

Remove Copyright headnote and myself from if_iwn.c, as noted in review.