Page MenuHomeFreeBSD

netmap: set IFCAP_NETMAP in if_capabilities
ClosedPublic

Authored by v.maffione_gmail.com on Nov 14 2018, 4:09 PM.

Details

Summary

On netmap_attach(), we need to set the IFCAP_NETMAP flag in ifp->if_capabilities.
Also remove the redundant code in cxgbe(4) that does the same.

Test Plan

Integration tests and unit tests ran successfully.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

np requested changes to this revision.Nov 26 2018, 5:57 PM

The cxgbe part of this diff reverts r309725. The commit message for r309725 indicates the changes were made to catch up with r307394, which was the a netmap update. Can you confirm that various releases of netmap have had different behavior regarding if_capabilities? If netmap has finally settled on handling the caps this way then I have no objection to one last catch-up in the driver.

This revision now requires changes to proceed.Nov 26 2018, 5:57 PM

The truth is that netmap not setting the IFCAP_NETMAP flag in ifp->if_capabilities after r307394 is a mistake that we did not notice so far.
It got introduced by refactoring, 3 years ago (see https://github.com/luigirizzo/netmap/commit/5d0796f93a1107eb14422c7b8ea416f7fd750a2e).
Sorry for that, it was unintended.
I just happened to notice that and fix it.

np accepted this revision.Nov 27 2018, 8:11 PM

The truth is that netmap not setting the IFCAP_NETMAP flag in ifp->if_capabilities after r307394 is a mistake that we did not notice so far.
It got introduced by refactoring, 3 years ago (see https://github.com/luigirizzo/netmap/commit/5d0796f93a1107eb14422c7b8ea416f7fd750a2e).
Sorry for that, it was unintended.
I just happened to notice that and fix it.

Ok, in that case can you revert r309725 all by itself as a separate commit? I think that will leave a better trail in svn history.

This revision is now accepted and ready to land.Nov 27 2018, 8:11 PM

Sounds good, that's a better approach. I'll do it as soon as @gnn or @hrs approve.

gnn accepted this revision.Nov 28 2018, 1:13 PM

I approve the plan laid out in the comments with np@

Closed by commit rS341144: netmap: set IFCAP_NETMAP in if_capabilities (authored by vmaffione, committed by ). · Explain WhyNov 28 2018, 2:08 PM
This revision was automatically updated to reflect the committed changes.