kern/127052: Still bridge issues - with L2 protocols such as PPPoE

Wed Sep 3 04:30:04 UTC 2008

The following reply was made to PR kern/127052; it has been noted by GNATS.

From: Eygene Ryabinkin <rea-fbsd at codelabs.ru>
To: Helge Oldach <freebsd-bridge-sep08 at oldach.net>
Cc: bug-followup at FreeBSD.org, philip at FreeBSD.org
Subject: Re: kern/127052: Still bridge issues - with L2 protocols such as
	PPPoE
Date: Wed, 3 Sep 2008 08:21:43 +0400

 --UNifc18z8z6e1QHx
 Content-Type: text/plain; charset=koi8-r
 Content-Disposition: inline
 Content-Transfer-Encoding: quoted-printable

 Tue, Sep 02, 2008 at 11:06:47PM +0200, Helge Oldach wrote:
 > Eygene supplied a patch that supposedly fixes this issue by introducing
 > a sysctl that makes the former if_bridge behaviour default, and which
 > must be turned on to enable MAC inheritance. I have not tested this
 > patch yet.

 And here is the patch itself:
 --- if_bridge-mac_inheritance.patch begins here ---
 =46rom 545d95995bb1879a6807be28a43d4ee061dda218 Mon Sep 17 00:00:00 2001
 =46rom: Eygene Ryabinkin <rea-fbsd at codelabs.ru>
 Date: Tue, 2 Sep 2008 19:49:44 +0400
 Subject: [PATCH] Add sysctl net.link.bridge.inherit_mac to control MAC inhe=
 ritance

 Philip Paeps enabled bridge to inherit its MAC from the first bridge
 member.  This broke ARP, it was fixed, but then Helge Oldach reported
 that this also brokes PPPoE when it is done on the bridged interface.

 I had implemented new sysctl that controls MAC inheritance.  It is off
 by default to enable previous behaviour of bridge until all problems
 with duplicated MAC addresses will be chased and fixed.

 Signed-off-by: Eygene Ryabinkin <rea-fbsd at codelabs.ru>
 ---
  sys/net/if_bridge.c |    9 +++++++--
  1 files changed, 7 insertions(+), 2 deletions(-)

 diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
 index a84a0ff..aee7c4a 100644
 --- a/sys/net/if_bridge.c
 +++ b/sys/net/if_bridge.c
 @@ -350,6 +350,7 @@ static int pfil_ipfw_arp =3D 0;   /* layer2 filter with=
  ipfw */
  static int pfil_local_phys =3D 0; /* run pfil hooks on the physical interf=
 ace for
                                     locally destined packets */
  static int log_stp   =3D 0;   /* log STP state changes */
 +static int bridge_inherit_mac =3D 0;   /* share MAC with first bridge memb=
 er */
  SYSCTL_INT(_net_link_bridge, OID_AUTO, pfil_onlyip, CTLFLAG_RW,
      &pfil_onlyip, 0, "Only pass IP packets when pfil is enabled");
  SYSCTL_INT(_net_link_bridge, OID_AUTO, ipfw_arp, CTLFLAG_RW,
 @@ -363,6 +364,9 @@ SYSCTL_INT(_net_link_bridge, OID_AUTO, pfil_local_phys,=
  CTLFLAG_RW,
      "Packet filter on the physical interface for locally destined packets"=
 );
  SYSCTL_INT(_net_link_bridge, OID_AUTO, log_stp, CTLFLAG_RW,
      &log_stp, 0, "Log STP state changes");
 +SYSCTL_INT(_net_link_bridge, OID_AUTO, inherit_mac, CTLFLAG_RW,
 +    &bridge_inherit_mac, 0,
 +    "Inherit MAC address from the first bridge member");
 =20
  struct bridge_control {
  	int	(*bc_func)(struct bridge_softc *, void *);
 @@ -921,7 +925,8 @@ bridge_delete_member(struct bridge_softc *sc, struct br=
 idge_iflist *bif,
  	 * the mac address of the bridge to the address of the next member, or
  	 * to its default address if no members are left.
  	 */
 -	if (!memcmp(IF_LLADDR(sc->sc_ifp), IF_LLADDR(ifs), ETHER_ADDR_LEN)) {
 +	if (bridge_inherit_mac &&
 +	    !memcmp(IF_LLADDR(sc->sc_ifp), IF_LLADDR(ifs), ETHER_ADDR_LEN)) {
  		if (LIST_EMPTY(&sc->sc_iflist))
  			bcopy(sc->sc_defaddr,
  			    IF_LLADDR(sc->sc_ifp), ETHER_ADDR_LEN);
 @@ -1028,7 +1033,7 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
  	 * member and the MAC address of the bridge has not been changed from
  	 * the default randomly generated one.
  	 */
 -	if (LIST_EMPTY(&sc->sc_iflist) &&
 +	if (bridge_inherit_mac && LIST_EMPTY(&sc->sc_iflist) &&
  	    !memcmp(IF_LLADDR(sc->sc_ifp), sc->sc_defaddr, ETHER_ADDR_LEN))
  		bcopy(IF_LLADDR(ifs), IF_LLADDR(sc->sc_ifp), ETHER_ADDR_LEN);
 =20
 --=20
 1.5.6.4
 --- if_bridge-mac_inheritance.patch ends here ---

 > I wonder what the purpose of MAC inheritance is anyway... Multiple
 > unicast MACs in one segment sounds pretty odd.

 As was explained to me by Philip Paeps,
 -----
 On 2008-08-15 18:24:29 (+0400), Eygene Ryabinkin <rea-fbsd at codelabs.ru> wro=
 te:
 > I wonder what was the real need of the commit r180140, where you added
 > preemption of first bridge member MAC address by the bridge itself?

 There were two reasons: firstly, it makes the bridge more predictable across
 reboots, particularly in setups using DHCP.  Secondly, this is the way the
 IEEE spec seems to suggest it should work.  It is also the way other bridgi=
 ng
 implementations I've encountered work -- which suggests my reading of the s=
 pec
 is correct.
 -----
 --=20
 Eygene
  _                ___       _.--.   #
  \`.|\..----...-'`   `-._.-'_.-'`   #  Remember that it is hard
  /  ' `         ,       __.--'      #  to read the on-line manual  =20
  )/' _/     \   `-_,   /            #  while single-stepping the kernel.
  `-'" `"\_  ,_.-;_.-\_ ',  fsc/as   #
      _.-'_./   {_.'   ; /           #    -- FreeBSD Developers handbook=20
     {_.-``-'         {_/            #

 --UNifc18z8z6e1QHx
 Content-Type: application/pgp-signature
 Content-Disposition: inline

 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v2.0.9 (FreeBSD)

 iEYEARECAAYFAki+ENcACgkQthUKNsbL7Yi/wgCgpyeZJSj2E5Bx7R8SdLN/gjRl
 DfMAnR76+UX8D/LtyeN8Upz2FNnufDZ9
 =J9Nn
 -----END PGP SIGNATURE-----

 --UNifc18z8z6e1QHx--