Page MenuHomeFreeBSD

netmap: Add field for carrying packet meta data
AbandonedPublic

Authored by thj on Apr 10 2024, 2:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 9, 5:11 PM
Unknown Object (File)
Oct 4 2024, 4:36 PM
Unknown Object (File)
Oct 4 2024, 11:56 AM
Unknown Object (File)
Oct 2 2024, 4:00 AM
Unknown Object (File)
Sep 30 2024, 9:56 PM
Unknown Object (File)
Sep 19 2024, 11:23 AM
Unknown Object (File)
Sep 7 2024, 10:47 PM
Unknown Object (File)
Sep 7 2024, 10:28 AM

Details

Reviewers
markj
vmaffione
shurd
Group Reviewers
iflib
Summary

Some drivers (such as ice) will strip vlan tags at device level and
provide this information as part of receive side meta data. Typically
this is bundled into the mbufs pkthdr.

We don't have the pkthdr available from netmap, add a field to the slot
to carry the vlan tag and add support to iflib based drivers to pass up
the vlan tag if available.

Test Plan

I've added support to vpp to reintegrate the vlan tag when copying in data from
netmap, they have the opposite problem and have no way to carry meta data with
a packet.

This survived multiple interfaces and a couple hundred GB of iperf.

I am not sure if this is the correct approach generally, my qualms are:

  • I may have missed an interface to disable the driver stripping the tag, but I can't get any ifconfig options to work the way I expect.
  • There may be another interface to carry this data for netmap, but I don't see it
  • It might be that this should be more general, but from reviewing pkthdr I didn't see other fields that would need this fix.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 57026
Build 53914: arc lint + arc unit

Event Timeline

thj requested review of this revision.Apr 10 2024, 2:37 PM
sys/net/netmap.h
167

Doesn't this break the userspace ABI? That is, all netmap applications would need to be recompiled to see the new layout of struct netmap_slot.

sys/net/netmap.h
167

Sadly it does break the ABI.

A different approach would be to reintegrate the vlan id into the packet stream as we moved data to the user ring, but that would take the overheard into iflib. I like this approach as it allows the application to handle the vlan without having to do additonal data movements, I'm not aware of another way to get the tag to userspace with the correct packet data.

sys/net/netmap.h
167

I can't comment on whether it's potentially acceptable to break the ABI this way. I wouldn't really expect so though. At the very least we should add explicit pad fields for the remaining 48 bits.

Aside from that, how would netmap applications know to check for an out-of-line vlan tag? Don't we at least need a flag to distinguish "no vlan tag" from "vlan id 0"?

I may have missed an interface to disable the driver stripping the tag, but I can't get any ifconfig options to work the way I expect.

Perhaps erj@ could comment on whether it's possible to avoid stripping the vlan tag?

Hi,

Netmap follows a KISS approach, and has not been designed to handle hardware offloads (or at least those offloads that require metadata to be stored in the mbuf).

In this way, you can keep the per packet-metadata (struct netmap_slot) very small (16 bytes) and 16-bytes aligned, to play efficiently with the processor cache layers.
These choices are what allows netmap to achive high packet rates with small packets.

Extending the netmap_slot like that would not be acceptable from my point of view: it breaks the ABI, it misalignes the ring, causing performance degradation.

The expected approach in your use-case would be to disable all the possible offloads in the hardware. If ice doesn't do that, maybe we should fix the ice driver.

In any case, the netmap_slot::ptrfield is only (optionally) used by VALE TX rings (see NS_INDIRECT). So the application can use it to store metadata, but that would be out of the ABI.

This revision now requires changes to proceed.Apr 11 2024, 2:50 PM

Thanks both for the comments, I didn't think this would be the correct approach, but it seemed easier to create a review for how I would like this to work.

I agree that the driver should really provide a work around, but this does have the benefit of letting the hardware do its thing while still making the vlan id available AND it avoids the overhead of integrating it into the stream to then take it out of the steam again.

@vmaffione would bundling the data to be carried with the slot into the ptr field and adding a flag be an acceptable way to add something like this?

Actually, netmap also has a more generic mechanism to store custom metadata within the netmap buffer.

If you look at the NETMAP_ROFFSET, NETMAP_WOFFSET, NETMAP_BUF_OFFSET macros and other, you will see that they allow to tell the driver that some space at the beginning of the buffer contains metadata, and actually RX or TX data starts at "offset" bytes from the beginning.
To store the per-packet offset, part the netmap_slot::ptr field is used. See also netmap_ring::offset_mask and netmap_ring::offset_mask.

This requires driver support (e.g. implementing NAF_OFFSET), which is already done in iflib (although some testing would be useful). The application needs to be aware of the offsets, though.

I have a temp fix for this directly in ice and will work with erj@ to have the driver support the vlan ifconfig flags correctly