On Thu, Jul 12, 2018 at 11:03 AM, Jay Vosburgh <jay.vosbu...@canonical.com> wrote: > Michal Soltys <sol...@ziu.info> wrote: > >>On 07/12/2018 04:51 PM, Jay Vosburgh wrote: >>> Mahesh Bandewar (महेश बंडेवार) wrote: >>> >>>> On Wed, Jul 11, 2018 at 3:23 PM, Michal Soltys <sol...@ziu.info> wrote: >>>>> >>>>> Hi, >>>>> >>>>> As weird as that sounds, this is what I observed today after bumping >>>>> kernel version. I have a setup where 2 bonds are attached to linux >>>>> bridge and physically are connected to two switches doing MSTP (and >>>>> linux bridge is just passing them). >>>>> >>>>> Initially I suspected some changes related to bridge code - but quick >>>>> peek at the code showed nothing suspicious - and the part of it that >>>>> explicitly passes stp frames if stp is not enabled has seen little >>>>> changes (e.g. per-port group_fwd_mask added recently). Furthermore - if >>>>> regular non-bonded interfaces are attached everything works fine. >>>>> >>>>> Just to be sure I detached the bond (802.3ad mode) and checked it with >>>>> simple tcpdump (ether proto \\stp) - and indeed no hello packets were >>>>> there (with them being present just fine on active enslaved interface, >>>>> or on the bond device in earlier kernels). >>>>> >>>>> If time permits I'll bisect tommorow to pinpoint the commit, but from >>>>> quick todays test - 4.9.x is working fine, while 4.16.16 (tested on >>>>> debian) and 4.17.3 (tested on archlinux) are failing. >>>>> >>>>> Unless this is already a known issue (or you have any suggestions what >>>>> could be responsible). >>>>> >>>> I believe these are link-local-multicast messages and sometime back a >>>> change went into to not pass those frames to the bonding master. This >>>> could be the side effect of that. >>> >>> Mahesh, I suspect you're thinking of: >>> >>> commit b89f04c61efe3b7756434d693b9203cc0cce002e >>> Author: Chonggang Li <chonggan...@google.com> >>> Date: Sun Apr 16 12:02:18 2017 -0700 >>> >>> bonding: deliver link-local packets with skb->dev set to link that >>> packets arrived on >>> >>> Michal, are you able to revert this patch and test? >>> >>> -J >>> >>> --- >>> -Jay Vosburgh, jay.vosbu...@canonical.com >>> >> >> >>Just tested - yes, reverting that patch solves the issues. > > Chonggang, > > Reading the changelog in your commit referenced above, I'm not > entirely sure what actual problem it is fixing. Could you elaborate? > > As the patch appears to cause a regression, it needs to be > either fixed or reverted. > > Mahesh, you signed-off on it as well, perhaps you also have some > context? >
I think the original idea behind it was to pass the LLDPDUs to the stack on the interface that they came on since this is considered to be link-local traffic and passing to bond-master would loose it's "linklocal-ness". This is true for LLDP and if you change the skb->dev of the packet, then you don't know which slave link it came on in (from LLDP consumer's perspective). I don't know much about STP but trunking two links and aggregating this link info through bond-master seems wrong. Just like LLDP, you are losing info specific to a link and the decision derived from that info could be wrong. Having said that, we determine "linklocal-ness" by looking at L2 and bondmaster shares this with lts slaves. So it does seem fair to pass those frames to the bonding-master but at the same time link-local traffic is supposed to be limited to the physical link (LLDP/STP/LACP etc). Your thoughts? > -J > > --- > -Jay Vosburgh, jay.vosbu...@canonical.com
