ip_encap: Recompute flowid for packets received over tunnel
Needs ReviewPublic
Actions

Authored by pouria on Fri, Oct 9, 8:29 PM.

Details

Reviewers

glebius
melifaro
adrian
gallatin
markj

Group Reviewers

network

Summary

Packets decapsulated by a tunnel interface carry the NIC's flowid
computed over the outer tunnel tuple, which is the same for every
flow a peer carries. Therefore mpath nhop selection places all
traffic on one path.

Add encap_set_flowid() to ip_encap.c, which rehashes a decapsulated
IP packet on its inner addresses.

The feature is off by default and is enabled with the per-vnet
net.route.hash_inbound sysctl.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77918
Build 74801: arc lint + arc unit

Event Timeline

pouria created this revision.Fri, Oct 9, 8:29 PM

Herald added subscribers: ae, imp. · View Herald TranscriptFri, Oct 9, 8:29 PM

pouria requested review of this revision.Fri, Oct 9, 8:29 PM

Harbormaster completed remote builds in B77918: Diff 189292.Fri, Oct 9, 8:29 PM

gallatin requested changes to this revision.Fri, Oct 9, 8:47 PM

gallatin added inline comments.

sys/netinet/ip_encap.c
448	Could this go into an inline wrapper, or simply be checked at the call-sites to avoid a function call when this is disabled?
456	I'd make it so this function cannot fail, for both performance and readability. Eg, rather than call m_pull(), just check that m->m_len >= sizeof(*ip), if not, leave the flowid intact. That removes calls to m_pullup() on every packet, and allows you to avoid all the m == NULL error handling at the call sites.
461	Note that you do not need this ifdef, rss hash calculations no longer depend on #ifdef RSS as of d9c55b2e8cd6b79f6926278e10a79f1bcca27a4b

This revision now requires changes to proceed.Fri, Oct 9, 8:47 PM

Address @gallatin comments. Thank you so much, it looks so much better now.

Harbormaster completed remote builds in B77924: Diff 189300.Fri, Oct 9, 9:28 PM

pouria added inline comments.Fri, Oct 9, 9:29 PM

sys/netinet/ip_encap.c
461	Note that you do not need this ifdef, rss hash calculations no longer depend on #ifdef RSS as of d9c55b2e8cd6b79f6926278e10a79f1bcca27a4b I was looking at if_gre.c:898 (gre_flowid), looks like I've to clean that up too.

pouria added inline comments.Fri, Oct 9, 9:40 PM

sys/netinet/ip_encap.c
461	Done: D60556

glebius added inline comments.Sat, Oct 10, 2:31 AM

sys/netinet/ip_encap.c
456	This is such a common pattern with m_pullup()! Here is what we want to do. In mbuf.h static inline struct mbuf * m_pullup(struct mbuf *m, u_int size) { if (__predict_false(m->m_len < size)) return (__m_pullup(m, size)); else return (m); } And in uipc_mbuf.c rename current function to `__m_pullup`. This is backward compatible and afterwards we can eliminate now extraneous checks for `m_len` as we touch other code.