From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 75665C388F9 for ; Thu, 19 Nov 2020 21:51:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0D2E922202 for ; Thu, 19 Nov 2020 21:51:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=waldekranz-com.20150623.gappssmtp.com header.i=@waldekranz-com.20150623.gappssmtp.com header.b="ZiE3I+tF" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726620AbgKSVvN (ORCPT ); Thu, 19 Nov 2020 16:51:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47536 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725877AbgKSVvM (ORCPT ); Thu, 19 Nov 2020 16:51:12 -0500 Received: from mail-lf1-x143.google.com (mail-lf1-x143.google.com [IPv6:2a00:1450:4864:20::143]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 28DA0C0613CF for ; Thu, 19 Nov 2020 13:51:12 -0800 (PST) Received: by mail-lf1-x143.google.com with SMTP id r9so10471510lfn.11 for ; Thu, 19 Nov 2020 13:51:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=waldekranz-com.20150623.gappssmtp.com; s=20150623; h=content-transfer-encoding:cc:subject:from:to:date:message-id :in-reply-to; bh=FqHFdAKYUfYkHxVShEHdSgGy2326k7gJbRPGeQmqbF4=; b=ZiE3I+tFcuRpn16OLWsH0ihrq/SsAkP6N2lhfpCb6AOsP4qaj1NOmYHJmDJ4DC0PDp XGAAxMhT/Wo7bIYwHFfBqCqUAi3EZVKKCL/P5ypWaTOSTFPcg+yjqmoPjA7dljfcIXuY y9tQAGAYPUcIt9PNFszZHBok0kdmAtfkKtet5oPLieXbUsNFmISUAUIpRQtIoejE/C60 C+iPmkuXwnFTc1LkECa12ceo4oOV4/Kp6BLWyBY6LJdVTffasyYvEq3IitpvbI99SSXe gm0s8BEQqTXbuTwcjnF13uieNTQULcO8RmA5yRHpoXvq/1n98G3ANqDR8/9lqydoR1jV PurA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:content-transfer-encoding:cc:subject:from:to :date:message-id:in-reply-to; bh=FqHFdAKYUfYkHxVShEHdSgGy2326k7gJbRPGeQmqbF4=; b=Ngq2ujIHJ7wHOqj99xJmA/GYrf5PHB7Ut9O0XI/AoHiicXcaKdIwPt+wb+xz3HvK6o z2tR2VBncMrPuslredkwpn7REJ0k0O00jD6Ta9PhKnblE7/DbDGSbliCAetvDPyI0HkC t/crjKrXORq35C4TMLpclxr30+oWdKOgmrwjkszKemApKKCq9xpIA+fa3cxZ9ev6ERaF mEvpsilg6lk887biZqqc+rd755sqLhVrQ3+4hgxe1rIH3pROqJ1vVsc5OFCsuinI/02N +cidc1eRImaEsS8qd2LpnOLbOuY9K643ilS/9f+0O9q+wmk7S7Q+UNNnJPWtpuwIfJId Sq0w== X-Gm-Message-State: AOAM532L4pu9p7WdbETb6VTLQzOwIESIxYxge4saxT5J5zYzvx1Ai3sS QOvHVUsXF0k6/lOMc4q3Eu9F3w== X-Google-Smtp-Source: ABdhPJykCZJ640TvBl3x3mmqUyEQtzeQO16eYZuSyOG3LY4smsCoCfsuhyKSkxH00/YgpcEdx/sJqQ== X-Received: by 2002:ac2:48b2:: with SMTP id u18mr7365952lfg.313.1605822670602; Thu, 19 Nov 2020 13:51:10 -0800 (PST) Received: from localhost (h-79-28.A259.priv.bahnhof.se. [79.136.79.28]) by smtp.gmail.com with ESMTPSA id m25sm104107lfh.205.2020.11.19.13.51.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 19 Nov 2020 13:51:09 -0800 (PST) Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Cc: , , , , , , , , Subject: Re: [PATCH net-next 1/4] net: bonding: Notify ports about their initial state From: "Tobias Waldekranz" To: "Jay Vosburgh" Date: Thu, 19 Nov 2020 22:20:34 +0100 Message-Id: In-Reply-To: <365.1605809882@famine> Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu Nov 19, 2020 at 11:18 AM CET, Jay Vosburgh wrote: > Tobias Waldekranz wrote: > > >When creating a static bond (e.g. balance-xor), all ports will always > >be enabled. This is set, and the corresponding notification is sent > >out, before the port is linked to the bond upper. > > > >In the offloaded case, this ordering is hard to deal with. > > > >The lower will first see a notification that it can not associate with > >any bond. Then the bond is joined. After that point no more > >notifications are sent, so all ports remain disabled. > > Why are the notifications generated in __netdev_upper_dev_link > (via bond_master_upper_dev_link) not sufficient? That notification only lets the switchdev driver know that the port is now a part of the bond; it says nothing about if the port is in the active transmit set or not. That notification actually is sent. Unfortunately this happens before the event your referencing, in the `switch (BOND_MODE(bond))` section just a few lines above. Essentially the conversation goes like this: bond0: swp0 has link and is in the active tx set. dsa: Cool, but swp0 is not a part of any LAG afaik; ignore. bond0: swp0 is now a part of bond0. dsa: OK, I'll set up the hardware, setting swp0 as inactive initially. This change just repeats the initial message at the end when the driver can make sense of it. Without it, modes that default ports to be inactive (e.g. LACP) still work, as the driver and the bond agree on the initial state in those cases. But for a static LAG, there will never be another event (until the link fails). > >This change simply sends an extra notification once the port has been > >linked to the upper to synchronize the initial state. > > > >Signed-off-by: Tobias Waldekranz > >--- > > drivers/net/bonding/bond_main.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_= main.c > >index 71c9677d135f..80c164198dcf 100644 > >--- a/drivers/net/bonding/bond_main.c > >+++ b/drivers/net/bonding/bond_main.c > >@@ -1897,6 +1897,8 @@ int bond_enslave(struct net_device *bond_dev, stru= ct net_device *slave_dev, > > goto err_unregister; > > } > >=20 > >+ bond_lower_state_changed(new_slave); > >+ > > res =3D bond_sysfs_slave_add(new_slave); > > if (res) { > > slave_dbg(bond_dev, slave_dev, "Error %d calling bond_sysfs_slave_add= \n", res); > > Would it be better to add this call further down, after all > possible failures have been checked? I.e., if this new call to > bond_lower_state_changed() completes, and then very soon afterwards the > upper is unlinked, could that cause any issues? All the work of configuring the LAG offload is done in the notification send out by netdev_upper_dev_link. So that will all have to be torn down in that case no matter where we place this call. So from the DSA/switchdev point-of-view, I would say no, and I believe these are the only consumers of the events. Additionally, I think it makes sense to place the call as early as possible as that means you have a smaller window of time where the bond and the switchdev driver may disagree on the port's state.