In D52782#1210730, @kgalazka wrote:

In D52782#1206718, @erj wrote:

Can you tell us what the hardware enhancements are? 👀

I added a bit more details.

(approval for the functionality; you should make @ziaee's suggested changes to the documentation)

Can you tell us what the hardware enhancements are? 👀

I think the automatic close fails if the diff is too big or has too many files? It's happened a lot to me, but I never figured out why. So as @kbowling says, it's not cutting corners, it's just an imperfect integration between git and Phabricator; it's normal to make small edits between the review being approved and pushing it upstream.

This all seems sensible to me.

I think it's ok as-is, but there are some future changes to look at.

I'm not familiar with QAT hardware, so these comments are mostly about documentation and trying to understand some of it a bit better.

In the description:

In D48812#1148812, @jhb wrote:

Do we still need this after fixing the PCI bus driver to stop caching the message count?

Cancelling a 4 port SFP OCP card sounds like an odd move to me.

It looks good to me, with the added newline

I'm going to assume there are some hidden #ifdefs that are getting stripped out that require the write() function to be more complex for some reason. @krzysztof.galazka_intel.com ?

I can now confirm that the I226-K doesn't need any extra changes to work properly; it links at 2.5G and DHCP works:

This means that the PSH and FIN flag correctly appear only in the last segment, but the CWR flag does not only appear in the first segment, but also in all middle segments.
Using this patch, this could be fixed.

When doing Accurate ECN as currently being specified in Accurate ECN, the masks should be:

mask.first	0xFF6
mask.middle	0xFF6
mask.last	0xFFF

The proposed patch would allow a NIC to be configured to do TSO for either classical ECN or accurate ECN.

Does this description make the motivation clear?

@Yogesh.Bhosale_intel.com I forgot to change the commit header to have you as the author; I'll fix that if/when I do the MFC.

In D46187#1053418, @zlei wrote:

In D46187#1052954, @erj wrote:

ixl(4) and ice(4) could integrate this function, too:

ixl(4) needs to remove the unused "vsi->oqdrops" variable and just not handle the IFCOUNTER_OQDROPS case in its switch statement in ixl_if_get_counter(), then call your new iflib_if_get_counter_default().

"vsi->oqdrops" is actually in use. It is set via IXL_SET_OQDROPS in function ixl_update_vsi_stats().

ixl(4) and ice(4) could integrate this function, too:

Is this supposed to help solve the problem in the PR? I'm not sure what's going on there, but I'm not sure it's clear in there except for your comment that Odrops isn't incrementing because they're not being retrieved properly.

I don't know if we'll see any other comments from anyone else on this, so I'll probably go and commit this sometime in the next couple days.

It would be nice if someone added a comment about what the operations in irdma_ctrl.c are doing, but I think I understand them and the replacements look ok to me.

It looks like it matches the cleanup that happens in the other return paths to me