From patchwork Mon Apr 6 21:58:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Arad, Ronen" X-Patchwork-Id: 458459 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 04D53140213 for ; Tue, 7 Apr 2015 07:58:36 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752956AbbDFV6b (ORCPT ); Mon, 6 Apr 2015 17:58:31 -0400 Received: from mga02.intel.com ([134.134.136.20]:29785 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752884AbbDFV6a (ORCPT ); Mon, 6 Apr 2015 17:58:30 -0400 Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 06 Apr 2015 14:58:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,534,1422950400"; d="scan'208";a="551781093" Received: from orsmsx107.amr.corp.intel.com ([10.22.240.5]) by orsmga003.jf.intel.com with ESMTP; 06 Apr 2015 14:58:29 -0700 Received: from orsmsx113.amr.corp.intel.com (10.22.240.9) by ORSMSX107.amr.corp.intel.com (10.22.240.5) with Microsoft SMTP Server (TLS) id 14.3.224.2; Mon, 6 Apr 2015 14:58:29 -0700 Received: from orsmsx101.amr.corp.intel.com ([169.254.8.47]) by ORSMSX113.amr.corp.intel.com ([169.254.7.45]) with mapi id 14.03.0224.002; Mon, 6 Apr 2015 14:58:29 -0700 From: "Arad, Ronen" To: Scott Feldman , "netdev@vger.kernel.org" CC: "jiri@resnulli.us" , "roopa@cumulusnetworks.com" , "linux@roeck-us.net" , "f.fainelli@gmail.com" , "Samudrala, Sridhar" Subject: RE: [PATCH net-next v3 19/26] switchdev: add new swdev_port_bridge_getlink Thread-Topic: [PATCH net-next v3 19/26] switchdev: add new swdev_port_bridge_getlink Thread-Index: AQHQbRxsGzjsbD2e3UqS4PReg1L4bp09CftQgAPuHwD//5DAMA== Date: Mon, 6 Apr 2015 21:58:28 +0000 Message-ID: References: <1427962212-18411-1-git-send-email-sfeldma@gmail.com> <1427962212-18411-20-git-send-email-sfeldma@gmail.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org >-----Original Message----- >From: Scott Feldman [mailto:sfeldma@gmail.com] >Sent: Monday, April 06, 2015 2:13 PM >To: Arad, Ronen >Cc: netdev@vger.kernel.org; jiri@resnulli.us; roopa@cumulusnetworks.com; >linux@roeck-us.net; f.fainelli@gmail.com; Samudrala, Sridhar >Subject: Re: [PATCH net-next v3 19/26] switchdev: add new >swdev_port_bridge_getlink > >On Sat, Apr 4, 2015 at 9:56 AM, Arad, Ronen wrote: >> >> >>>-----Original Message----- >>>From: sfeldma@gmail.com [mailto:sfeldma@gmail.com] >>>Sent: Thursday, April 02, 2015 1:10 AM >>>To: netdev@vger.kernel.org >>>Cc: jiri@resnulli.us; roopa@cumulusnetworks.com; linux@roeck-us.net; >>>f.fainelli@gmail.com; Samudrala, Sridhar; Arad, Ronen >>>Subject: [PATCH net-next v3 19/26] switchdev: add new >>>swdev_port_bridge_getlink >>> >>>From: Scott Feldman >>> >>>Like bridge_setlink, add swdev wrapper to handle bridge_getlink and call >into >>>port driver to get port attrs. For now, only BR_LEARNING and >BR_LEARNING_SYNC >>>are returned. To add more, we'll probably want to break away from >>>ndo_dflt_bridge_getlink() and build the netlink skb directly in the swdev >>>code. > >[cut] > >> >> swdev_port_bridge_getlink and the function it wraps >> ndo_dflt_bridge_getlink are only useful when all you need is to return >> a set of brport attributes. >> When a switchdev driver supports VLAN filtering without being enslaved >> to a bridge this no longer works. ndo_dflt_bridge_getlink will end up in >> a netlink RTM_NEWLINK message without the VLAN filtering info which is >> maintained internally by the switchdev driver. >> The driver will have to duplicate the code in ndo_dflt_bridge_getlink in >> order to return a single RTM_NEWLINK message per port. >> Can we break ndo_dflt_bridge_getlink to components that could be used >> by such driver? >> It would be useful to have one function that could fill the >> IFLA_PROTINFO and another one for the IFLA_AF_SPEC. > >I couldn't find a consumer of the RTM_NEWLINK msg returned from >RTM_GETLINK that wants the VLAN part of afspec. Did I miss one? It is in my driver which is not up-streamed yet. I don't know if we really need to make any changes in the kernel. It could be appropriate to generate two separate netlink messages from the driver for a single port: 1) What ndo_dflt_bridge_getlink returns. This consists of IFLA_AF_SPEC With bridge mode and self flag and IFLA_PROTINFO with bridge flags. 2) A second message with VLAN table in IFLA_AF_SPEC. This is what the bridge module would have returned had the port been enslaved to a bridge. It seems that "bridge vlan" command had to cope with that anyway. I made a change to iproute2 bridge command such that is defers printing the interface name until it encounters the first IFLA_AF_SPEC which includes an IFLA_BRIDGE_VLAN_INFO attribute instead of doing it unconditionally for every IFLA_AF_SPEC. Here is my suggested iproute2 patch. I can post it if the suggested handling of AF_BRIDGE getlink (as I described above) is acceptable/ desirable. fprintf(fp, "\n"); } } - fprintf(fp, "\n"); + if (vlan_table_found) + fprintf(fp, "\n"); fflush(fp); return 0; } > >-scott diff --git a/bridge/vlan.c b/bridge/vlan.c index 2ae739c..de423b9 100644 --- a/bridge/vlan.c +++ b/bridge/vlan.c @@ -145,6 +145,7 @@ static int print_vlan(const struct sockaddr_nl *who, struct ifinfomsg *ifm = NLMSG_DATA(n); int len = n->nlmsg_len; struct rtattr * tb[IFLA_MAX+1]; + bool vlan_table_found = false; if (n->nlmsg_type != RTM_NEWLINK) { fprintf(stderr, "Not RTM_NEWLINK: %08x %08x %08x\n", @@ -174,13 +175,22 @@ static int print_vlan(const struct sockaddr_nl *who, struct rtattr *i, *list = tb[IFLA_AF_SPEC]; int rem = RTA_PAYLOAD(list); - fprintf(fp, "%s", ll_index_to_name(ifm->ifi_index)); for (i = RTA_DATA(list); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) { struct bridge_vlan_info *vinfo; if (i->rta_type != IFLA_BRIDGE_VLAN_INFO) continue; : + /* Printing interface name is deferred until first + * IFLA_BRIDGE_VLAN_INFO is seen since there could be + * IFLA_AF_SPEC with only IFLA_BRIDGE_MODE or/and + * IFLA_BRIDGE_FLAGS + */ + if (!vlan_table_found) { + vlan_table_found = true; + fprintf(fp, "%s", ll_index_to_name(ifm->ifi_inde+ } + vinfo = RTA_DATA(i); if (vinfo->flags & BRIDGE_VLAN_INFO_RANGE_END) fprintf(fp, "-%hu", vinfo->vid); @@ -195,7 +205,8 @@ static int print_vlan(const struct sockaddr_nl *who,