Page MenuHomeFreeBSD

if_vxlan(4): Allow netmap_generic to intercept RX packets.
ClosedPublic

Authored by afedorov on Jun 4 2021, 4:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jul 10, 11:32 AM
Unknown Object (File)
Sat, Jul 6, 1:51 PM
Unknown Object (File)
Wed, Jul 3, 10:08 PM
Unknown Object (File)
Jun 16 2024, 2:31 AM
Unknown Object (File)
Jun 15 2024, 6:37 PM
Unknown Object (File)
Jun 5 2024, 4:22 PM
Unknown Object (File)
May 27 2024, 2:26 AM
Unknown Object (File)
May 22 2024, 8:16 PM

Details

Summary

Netmap (generic) intercepts the if_input method to handle RX packets. So:

  • Call ifp->if_input() instead of netisr_dispatch().
  • Add stricter check for incoming packet length.

This change is very useful with bhyve + vale + if_vxlan.

We have been using this patch for several years.

Test Plan

Setup the following configuration:
Host 1: bhyve VM - vale(4) switch - if_vxlan(4)
Host 2: bhyve VM - vale(4) switch - if_vxlan(4)

Before applying the patch, there is no network connection between the VMs.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

donner added a subscriber: donner.
donner added inline comments.
sys/net/if_vxlan.c
2866

So we never get an error indication if the packet could not be transmitted?

This revision is now accepted and ready to land.Jun 4 2021, 4:07 PM
sys/net/if_vxlan.c
2866

Unfortunately yes. But the calling code doesn't handle errors either.

2792         vni = ntohl(vxh->vxlh_vni) >> VXLAN_HDR_VNI_SHIFT;
2793 
2794         /* Adjust to the start of the inner Ethernet frame. */
2795         m_adj_decap(m, offset + sizeof(struct vxlan_header));
2796 
2797         error = vxlan_input(vso, vni, &m, srcsa);
2798         MPASS(error != 0 || m == NULL);
2799 
2800 out:
2801         if (m != NULL)
2802                 m_freem(m);
2803 }
kib added inline comments.
sys/net/if_vxlan.c
2774

I would place this check into vxlan_input(), since that is the function that access inner ethernet header.

2867–2868

And I would set error = 0 there.

I know it's not directly related but the 'if' here is not needed. If you're tightening the len checks in the function then maybe clean this too?

out:

if (m != NULL)
        m_freem(m);
  • Move ethernet packet length check to vxlan_input().
This revision now requires review to proceed.Jun 5 2021, 9:10 AM
In D30638#688320, @np wrote:

I know it's not directly related but the 'if' here is not needed. If you're tightening the len checks in the function then maybe clean this too?

out:

if (m != NULL)
        m_freem(m);

Do you mean 'out' label in vxlan_rcv_udp_packet()?

If yes, then I'm not sure, because vxlan_input() can return an error without touching the mbuf.

vmaffione added inline comments.
sys/net/if_vxlan.c
2866

However, the calling code may be improved in the future. And additional callers may be added. Why are we removing an error propagation code path?
I think a function should return the appropriate return value irrespective of the caller ignoring that. It's more future proof.

This revision now requires changes to proceed.Jun 6 2021, 4:16 PM
sys/net/if_vxlan.c
2866

Unfortunately, the if_input() ( by default: ether_input() ) method does not return an error.

void    (*if_input)

There are many drivers that call if_input () directly. Even netmap calls if_input() and doesn't handle the error.

Is this error handling really necessary for if_vxlan?

sys/net/if_vxlan.c
2866

Error handling is not necessary at this point and within this review.

Sounds good. I had not noticed that *if_input returns void.

This revision is now accepted and ready to land.Jun 16 2021, 9:28 PM