> This looks better than what I remember from the previous version. Are > the bot's complains legitime?
Yes, the Critical one is real. veth creates and deletes its NAPI instances based on wanted_features (veth_gro_requested() in veth_open() and veth_close()). This patch only clears NETIF_F_GRO from the active features. So after the attach, wanted_features still says "GRO on" while the feature is off. Two things can go wrong because of that: - The member is up with GRO on, then gets attached. veth_set_features() deletes the NAPIs. Later the member is set down and veth_close() deletes them a second time. napi_disable() then waits forever. - The member is down with GRO on, gets attached, is set up, and is then detached while still up. veth_open() created the NAPIs, and the detach creates them again. That ends in the BUG_ON() in napi_enable(). The v7 selftests always attach and detach the member while it is up, so they did not see either case. I already replied to the bot with "pw-bot: cr", but by mistake under the patch 4 report. My plan for v8: keep the persistent filter, but at the same place also clear GRO from the member's wanted_features. This is what netdev_sync_lower_features() does for the UPPER_DISABLES bits. Then requested and active GRO always agree, and veth does not need a change. The downside: after the member is detached, GRO stays off, and the user has to enable it again. LRO behaves the same way today. If you or the other maintainers prefer that veth is fixed instead, so that it looks at the active feature, I can do that. The Low one is only a comment problem. For stacked members (bond, macvlan, ...) GRO runs on the lower device, so "software GRO must be off before frames can arrive" is only true for direct members. Patch 3 segments any GSO input anyway. I will fix the comment. -- Xin
