if_epair: delete mbuf tags
ClosedPublic
Actions

Authored by kp on Oct 26 2021, 9:53 AM.

Details

Reviewers

bz
glebius

Group Reviewers

network
pfsense

Commits

rG6d20b6de6ae2: if_epair: delete mbuf tags
rGae23f081c5c7: if_epair: delete mbuf tags
rG62d2dcafb7f3: if_epair: delete mbuf tags

Summary

Remove all (non-persistent) tags when we transmit a packet. Real network
interfaces do not carry any tags either, and leaving tags attached can
produce unexpected results.

MFC after: 3 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 42402
Build 39290: arc lint + arc unit

Event Timeline

kp created this revision.Oct 26 2021, 9:53 AM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptOct 26 2021, 9:53 AM

kp requested review of this revision.Oct 26 2021, 9:53 AM

Harbormaster completed remote builds in B42372: Diff 97465.Oct 26 2021, 9:53 AM

kp added a child revision: D32664: mbuf: PACKET_TAG_PF should not be persistent.Oct 26 2021, 9:53 AM

bz added a subscriber: bz.Oct 26 2021, 10:15 AM

bz added inline comments.

sys/net/if_epair.c

459

Not sure if the other review we were discussion already has this but in my working branch I also have (note: the code looks different so the surroundings are different):

...
    157                         /* Remove any CSUM_SND_TAG if there as ether_input will barf. */
    158                         if (m->m_pkthdr.csum_flags & CSUM_SND_TAG) {
    159                                 m_snd_tag_rele(m->m_pkthdr.snd_tag);
    160                                 m->m_pkthdr.snd_tag = NULL;
    161                                 m->m_pkthdr.csum_flags &= ~CSUM_SND_TAG;
    162                         }
    163                         if_setrcvif(m, ifp);
    164                         M_SETFIB(m, ifp->if_fib);
    165                         MPASS((m->m_pkthdr.csum_flags & CSUM_SND_TAG) == 0);
...

I don't think m_tag_delete_nonpersistent() takes send tags into account?

zlei added a subscriber: zlei.Oct 27 2021, 5:50 AM

Also clear send tags

Harbormaster completed remote builds in B42398: Diff 97536.Oct 27 2021, 10:01 AM

bz added inline comments.Oct 27 2021, 2:14 PM

sys/net/if_epair.c
563	Given you do the same in epair_start_locked() can we move the call down after the if() block below to save us a tiny little work in an already pressing situation?