Bug 253069 - cxgbe fl_pktshift not handled in netmap mode
Summary: cxgbe fl_pktshift not handled in netmap mode
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.2-RELEASE
Hardware: amd64 Any
: --- Affects Only Me
Assignee: Navdeep Parhar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-28 22:56 UTC by Brian Poole
Modified: 2024-01-12 17:56 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Poole 2021-01-28 22:56:33 UTC
Hello,

Today I tried using the hw.cxgbe.fl_pktshift sysctl to see if it would improve the overall packet rate. I was using hw.cxgbe.fl_pktshift=2 to produce 4 byte alignment with my Ethernet/IP data. After looking at the data, I realized the Ethernet frames were not being recognized. I am using the cc0 interface in netmap mode.

I captured packets from the interface and viewed them using tcpdump. It looks like the pktshift is not being accounted for by the driver as the first two byte are the EtherType from the Ethernet header.

17:42:12.555105 1a:84:9a:83:ca:44 > 00:00:8b:cb:f9:5d, ethertype Unknown (0x268f), length 811: 
        0x0000:  0800 4500 031d 0001 0000 4011 07bd 8fdb

I reviewed the driver code and found this segment in eth_rx():

    m0->m_pkthdr.len -= sc->params.sge.fl_pktshift;
    m0->m_len -= sc->params.sge.fl_pktshift;
    m0->m_data += sc->params.sge.fl_pktshift;

So the driver is transparently removing the shift from the length and the data pointer. However, in service_nm_rxq() I see only:

    case CPL_RX_PKT:
                ring->slot[fl_cidx].len = G_RSPD_LEN(lq) -
                    sc->params.sge.fl_pktshift;

The length is being adjusted. However does the data pointer need adjusting as well? Is this sysctl not supported while in netmap mode?
Comment 1 Navdeep Parhar freebsd_committer freebsd_triage 2021-04-28 20:15:05 UTC
I'm not aware of any way to specify to netmap that the received frame starts at
some offset from the start of the buffer.

Vincenzo, is it possible to do this?  Otherwise we should just document that
fl_pktshift = 0 is the only valid setting when in netmap mode.
Comment 2 Brian Poole 2021-05-26 00:56:16 UTC
Navdeep,

Thank you for your response. This was just an experiment and not a technique I had settled on using. Since posting this issue, I've seen two reviews by vmaffione related to netmap offsets. I am referring to:

netmap: add kernel support for the "offsets" feature
https://reviews.freebsd.org/R10:a6d768d845c173823785c71bb18b40074e7a8998
iflib: add support for netmap offsets
https://reviews.freebsd.org/R10:361e950180025b72cf78a41a3563d32f9beb0b05

From my cursory reading of these, I believe the cxgbe driver could adopt support similar to iflib and report NAF_OFFSETS to handle buffers that do not start at the beginning of the allocated memory?

Again, initially I was confused by the invalid packets. I am perfectly happy if a shift is not allowed when in netmap mode. Alternatively, if the shift can be integrated with netmap using NAF_OFFSETS, that sounds like a great plan to me too. Thank for for your support of the cxgbe driver!
Comment 3 Vincenzo Maffione freebsd_committer freebsd_triage 2021-06-02 08:24:32 UTC
Hi,
  Since NAF_OFFSET was introduced some weeks ago, it is possible to use the reserved field in the netmap_slot struct to specify an offset in the netmap buffer for both tx and tx. This is meant to ease header push/pop operations, but it can also be used to improve alignment. Each driver needs to support the offset feature in both txsync and rxsync routines. The changes are however usually very simple. We could set up a review for that.
Comment 4 commit-hook freebsd_committer freebsd_triage 2024-01-08 20:07:38 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=df8a58b17a1907ed3b4597475d1cb8eacc9636de

commit df8a58b17a1907ed3b4597475d1cb8eacc9636de
Author:     Navdeep Parhar <np@FreeBSD.org>
AuthorDate: 2024-01-05 01:39:31 +0000
Commit:     Navdeep Parhar <np@FreeBSD.org>
CommitDate: 2024-01-08 20:04:07 +0000

    cxgbe(4): Add support for netmap offsets.

    PR:             253069
    MFC after:      1 week
    Sponsored by:   Chelsio Communications

 sys/dev/cxgbe/t4_netmap.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-01-11 05:34:14 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c4bac077581ee38a3c64cd3a76cb9dd974dd749b

commit c4bac077581ee38a3c64cd3a76cb9dd974dd749b
Author:     Navdeep Parhar <np@FreeBSD.org>
AuthorDate: 2024-01-05 01:39:31 +0000
Commit:     Navdeep Parhar <np@FreeBSD.org>
CommitDate: 2024-01-11 05:22:31 +0000

    cxgbe(4): Add support for netmap offsets.

    PR:             253069
    Sponsored by:   Chelsio Communications

    (cherry picked from commit df8a58b17a1907ed3b4597475d1cb8eacc9636de)

 sys/dev/cxgbe/t4_netmap.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)