Changeset View
Standalone View
sys/net/if_bridge.c
Show First 20 Lines • Show All 2,533 Lines • ▼ Show 20 Lines | if (m->m_flags & (M_BCAST|M_MCAST)) { | ||||
return (m); | return (m); | ||||
} | } | ||||
if ((bif->bif_flags & IFBIF_STP) && | if ((bif->bif_flags & IFBIF_STP) && | ||||
bif->bif_stp.bp_state == BSTP_IFSTATE_DISCARDING) { | bif->bif_stp.bp_state == BSTP_IFSTATE_DISCARDING) { | ||||
return (m); | return (m); | ||||
} | } | ||||
#if (defined(INET) || defined(INET6)) | #if defined(INET) || defined(INET6) | ||||
# define OR_CARP_CHECK_WE_ARE_DST(iface) \ | #define CARP_CHECK_WE_ARE_DST(iface) \ | ||||
|| ((iface)->if_carp \ | ((iface)->if_carp && (*carp_forus_p)((iface), eh->ether_dhost)) | ||||
&& (*carp_forus_p)((iface), eh->ether_dhost)) | #define CARP_CHECK_WE_ARE_SRC(iface) \ | ||||
# define OR_CARP_CHECK_WE_ARE_SRC(iface) \ | ((iface)->if_carp && (*carp_forus_p)((iface), eh->ether_shost)) | ||||
|| ((iface)->if_carp \ | |||||
&& (*carp_forus_p)((iface), eh->ether_shost)) | |||||
#else | #else | ||||
# define OR_CARP_CHECK_WE_ARE_DST(iface) | #define CARP_CHECK_WE_ARE_DST(iface) false | ||||
# define OR_CARP_CHECK_WE_ARE_SRC(iface) | #define CARP_CHECK_WE_ARE_SRC(iface) false | ||||
#endif | #endif | ||||
#ifdef INET6 | #ifdef INET6 | ||||
# define OR_PFIL_HOOKED_INET6 \ | #define PFIL_HOOKED_INET6 PFIL_HOOKED_IN(V_inet6_pfil_head) | ||||
|| PFIL_HOOKED_IN(V_inet6_pfil_head) | |||||
#else | #else | ||||
# define OR_PFIL_HOOKED_INET6 | #define PFIL_HOOKED_INET6 false | ||||
#endif | #endif | ||||
#define GRAB_OUR_PACKETS(iface) \ | #define GRAB_OUR_PACKETS(iface) \ | ||||
if ((iface)->if_type == IFT_GIF) \ | if ((iface)->if_type == IFT_GIF) \ | ||||
continue; \ | continue; \ | ||||
/* It is destined for us. */ \ | /* It is destined for us. */ \ | ||||
if (memcmp(IF_LLADDR((iface)), eh->ether_dhost, ETHER_ADDR_LEN) == 0 \ | if (memcmp(IF_LLADDR(iface), eh->ether_dhost, ETHER_ADDR_LEN) == 0 || \ | ||||
OR_CARP_CHECK_WE_ARE_DST((iface)) \ | CARP_CHECK_WE_ARE_DST(iface)) { \ | ||||
melifaro: It's much better than it was before.
Any thoughts on making it an inline function to improve… | |||||
Done Inline Actions
Are you thinking of STP learning? This is a different type of learning, and by default is always enabled. I suspect that it is useful to cache at least vlan, in any case. I tried implementing your suggestion, but I don't know if it's much better. The problem is that there are three possible return values: 1) continue processing in ether_input(), 2) drop the packet, 3) try the next interface. So to make the control flow nice, I ended up introducing a couple of helpers. See https://reviews.freebsd.org/D38425 markj: > It seems that vlan and sc are only used when the interface is in learning mode (e.g. short… | |||||
Not Done Inline Actions
Yep, that's datapath learning and I was wrong.
I'm not advocating for my approach, more trying to discuss a reasonable approach to eliminate the huge #defined function.
Thank you for doing that! melifaro: > Are you thinking of STP learning? This is a different type of learning, and by default is… | |||||
Not Done Inline ActionsBtw, do you mind adding kp / network to the new review so they can comment/compare as well? melifaro: Btw, do you mind adding kp / network to the new review so they can comment/compare as well? | |||||
) { \ | |||||
if (bif->bif_flags & IFBIF_LEARNING) { \ | if (bif->bif_flags & IFBIF_LEARNING) { \ | ||||
error = bridge_rtupdate(sc, eh->ether_shost, \ | error = bridge_rtupdate(sc, eh->ether_shost, \ | ||||
vlan, bif, 0, IFBAF_DYNAMIC); \ | vlan, bif, 0, IFBAF_DYNAMIC); \ | ||||
if (error && bif->bif_addrmax) { \ | if (error && bif->bif_addrmax) { \ | ||||
m_freem(m); \ | m_freem(m); \ | ||||
return (NULL); \ | return (NULL); \ | ||||
} \ | } \ | ||||
} \ | } \ | ||||
m->m_pkthdr.rcvif = iface; \ | m->m_pkthdr.rcvif = iface; \ | ||||
if ((iface) == ifp) { \ | if ((iface) == ifp) { \ | ||||
/* Skip bridge processing... src == dest */ \ | /* Skip bridge processing... src == dest */ \ | ||||
return (m); \ | return (m); \ | ||||
} \ | } \ | ||||
/* It's passing over or to the bridge, locally. */ \ | /* It's passing over or to the bridge, locally. */ \ | ||||
ETHER_BPF_MTAP(bifp, m); \ | ETHER_BPF_MTAP(bifp, m); \ | ||||
if_inc_counter(bifp, IFCOUNTER_IPACKETS, 1); \ | if_inc_counter(bifp, IFCOUNTER_IPACKETS, 1); \ | ||||
if_inc_counter(bifp, IFCOUNTER_IBYTES, m->m_pkthdr.len); \ | if_inc_counter(bifp, IFCOUNTER_IBYTES, m->m_pkthdr.len); \ | ||||
/* Filter on the physical interface. */ \ | /* Filter on the physical interface. */ \ | ||||
if (V_pfil_local_phys && (PFIL_HOOKED_IN(V_inet_pfil_head) \ | if (V_pfil_local_phys && (PFIL_HOOKED_IN(V_inet_pfil_head) || \ | ||||
OR_PFIL_HOOKED_INET6)) { \ | PFIL_HOOKED_INET6)) { \ | ||||
if (bridge_pfil(&m, NULL, ifp, \ | if (bridge_pfil(&m, NULL, ifp, \ | ||||
PFIL_IN) != 0 || m == NULL) { \ | PFIL_IN) != 0 || m == NULL) { \ | ||||
return (NULL); \ | return (NULL); \ | ||||
} \ | } \ | ||||
} \ | } \ | ||||
if ((iface) != bifp) \ | if ((iface) != bifp) \ | ||||
ETHER_BPF_MTAP(iface, m); \ | ETHER_BPF_MTAP(iface, m); \ | ||||
return (m); \ | return (m); \ | ||||
} \ | } \ | ||||
\ | \ | ||||
/* We just received a packet that we sent out. */ \ | /* We just received a packet that we sent out. */ \ | ||||
if (memcmp(IF_LLADDR((iface)), eh->ether_shost, ETHER_ADDR_LEN) == 0 \ | if (memcmp(IF_LLADDR(iface), eh->ether_shost, ETHER_ADDR_LEN) == 0 || \ | ||||
OR_CARP_CHECK_WE_ARE_SRC((iface)) \ | CARP_CHECK_WE_ARE_SRC(iface)) { \ | ||||
) { \ | |||||
m_freem(m); \ | m_freem(m); \ | ||||
return (NULL); \ | return (NULL); \ | ||||
} | } | ||||
/* | /* | ||||
* Unicast. Make sure it's not for the bridge. | * Unicast. Make sure it's not for the bridge. | ||||
*/ | */ | ||||
do { GRAB_OUR_PACKETS(bifp) } while (0); | do { GRAB_OUR_PACKETS(bifp) } while (0); | ||||
/* | /* | ||||
* Give a chance for ifp at first priority. This will help when the | * Give a chance for ifp at first priority. This will help when the | ||||
* packet comes through the interface like VLAN's with the same MACs | * packet comes through the interface like VLAN's with the same MACs | ||||
* on several interfaces from the same bridge. This also will save | * on several interfaces from the same bridge. This also will save | ||||
* some CPU cycles in case the destination interface and the input | * some CPU cycles in case the destination interface and the input | ||||
* interface (eq ifp) are the same. | * interface (eq ifp) are the same. | ||||
*/ | */ | ||||
do { GRAB_OUR_PACKETS(ifp) } while (0); | do { GRAB_OUR_PACKETS(ifp) } while (0); | ||||
/* Now check the all bridge members. */ | /* Now check the all bridge members. */ | ||||
CK_LIST_FOREACH(bif2, &sc->sc_iflist, bif_next) { | CK_LIST_FOREACH(bif2, &sc->sc_iflist, bif_next) { | ||||
GRAB_OUR_PACKETS(bif2->bif_ifp) | GRAB_OUR_PACKETS(bif2->bif_ifp) | ||||
} | } | ||||
#undef OR_CARP_CHECK_WE_ARE_DST | #undef CARP_CHECK_WE_ARE_DST | ||||
#undef OR_CARP_CHECK_WE_ARE_SRC | #undef CARP_CHECK_WE_ARE_SRC | ||||
#undef OR_PFIL_HOOKED_INET6 | #undef PFIL_HOOKED_INET6 | ||||
#undef GRAB_OUR_PACKETS | #undef GRAB_OUR_PACKETS | ||||
/* Perform the bridge forwarding function. */ | /* Perform the bridge forwarding function. */ | ||||
bridge_forward(sc, bif, m); | bridge_forward(sc, bif, m); | ||||
return (NULL); | return (NULL); | ||||
} | } | ||||
▲ Show 20 Lines • Show All 1,157 Lines • Show Last 20 Lines |
It's much better than it was before.
Any thoughts on making it an inline function to improve readability further?
There are 8 arguments that are used in the macro: iface, eh, bif, m, ifp, bifp, vlan and sc.
It seems that vlan and sc are only used when the interface is in learning mode (e.g. short period of time), so these can be derived from m and ifp, leaving 6 arguments, which can be doable.
For example, something like
bool grab_our_packet(target_ifp, source_ifp, source_bifp, source_bif, &m, eh) {} returning true if the packet is consumed can be an option.
what do you think?