Re: [Bridge] [PATCH v2 net-next 0/7] net: Plumb extack error reporting to enslavements

[David Miller]

From: David Ahern 
Date: Wed,  4 Oct 2017 17:48:44 -0700

> Another round of extending extack error reporting, this time for
> enslavements through ndo_add_slave and notifiers.
> 
> v2
> - changed how the messages are added to bonding driver per Jiri's request
> - fixed spectrum message for LAG overflow per Ido's comment

Series applied, thanks David.

[Bridge] [PATCH v2 net-next 7/7] mlxsw: spectrum: Add extack messages for enslave failures

[David Ahern]

mlxsw fails device enslavement for a number of reasons. Use the extack
facility to return an error message to the user stating why the enslave
is failing.

Messages are prefixed with "spectrum" so users know it is a constraint
imposed by the hardware driver. For example:
$ ip li add br0.11 link br0 type vlan id 11
$ ip li set swp11 master br0
Error: spectrum: Enslaving a port to a device that already has an upper 
device is not supported.

Signed-off-by: David Ahern 
Reviewed-by: Ido Schimmel 
Tested-by: Ido Schimmel 
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 47 --
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c 
b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 3adf237c951a..5cd4df08ce97 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4019,14 +4019,21 @@ static int mlxsw_sp_lag_index_get(struct mlxsw_sp 
*mlxsw_sp,
 static bool
 mlxsw_sp_master_lag_check(struct mlxsw_sp *mlxsw_sp,
  struct net_device *lag_dev,
- struct netdev_lag_upper_info *lag_upper_info)
+ struct netdev_lag_upper_info *lag_upper_info,
+ struct netlink_ext_ack *extack)
 {
u16 lag_id;
 
-   if (mlxsw_sp_lag_index_get(mlxsw_sp, lag_dev, _id) != 0)
+   if (mlxsw_sp_lag_index_get(mlxsw_sp, lag_dev, _id) != 0) {
+   NL_SET_ERR_MSG(extack,
+  "spectrum: Exceeded number of supported LAG 
devices");
return false;
-   if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
+   }
+   if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
+   NL_SET_ERR_MSG(extack,
+  "spectrum: LAG device using unsupported Tx 
type");
return false;
+   }
return true;
 }
 
@@ -4231,6 +4238,7 @@ static int mlxsw_sp_netdevice_port_upper_event(struct 
net_device *lower_dev,
 {
struct netdev_notifier_changeupper_info *info;
struct mlxsw_sp_port *mlxsw_sp_port;
+   struct netlink_ext_ack *extack;
struct net_device *upper_dev;
struct mlxsw_sp *mlxsw_sp;
int err = 0;
@@ -4238,6 +4246,7 @@ static int mlxsw_sp_netdevice_port_upper_event(struct 
net_device *lower_dev,
mlxsw_sp_port = netdev_priv(dev);
mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
info = ptr;
+   extack = netdev_notifier_info_to_extack(>info);
 
switch (event) {
case NETDEV_PRECHANGEUPPER:
@@ -4245,25 +4254,43 @@ static int mlxsw_sp_netdevice_port_upper_event(struct 
net_device *lower_dev,
if (!is_vlan_dev(upper_dev) &&
!netif_is_lag_master(upper_dev) &&
!netif_is_bridge_master(upper_dev) &&
-   !netif_is_ovs_master(upper_dev))
+   !netif_is_ovs_master(upper_dev)) {
+   NL_SET_ERR_MSG(extack,
+  "spectrum: Unknown upper device type");
return -EINVAL;
+   }
if (!info->linking)
break;
-   if (netdev_has_any_upper_dev(upper_dev))
+   if (netdev_has_any_upper_dev(upper_dev)) {
+   NL_SET_ERR_MSG(extack,
+  "spectrum: Enslaving a port to a device 
that already has an upper device is not supported");
return -EINVAL;
+   }
if (netif_is_lag_master(upper_dev) &&
!mlxsw_sp_master_lag_check(mlxsw_sp, upper_dev,
-  info->upper_info))
+  info->upper_info, extack))
return -EINVAL;
-   if (netif_is_lag_master(upper_dev) && vlan_uses_dev(dev))
+   if (netif_is_lag_master(upper_dev) && vlan_uses_dev(dev)) {
+   NL_SET_ERR_MSG(extack,
+  "spectrum: Master device is a LAG master 
and this device has a VLAN");
return -EINVAL;
+   }
if (netif_is_lag_port(dev) && is_vlan_dev(upper_dev) &&
-   !netif_is_lag_master(vlan_dev_real_dev(upper_dev)))
+   !netif_is_lag_master(vlan_dev_real_dev(upper_dev))) {
+   NL_SET_ERR_MSG(extack,
+  "spectrum: Can not put a VLAN on a LAG 
port");
return -EINVAL;
-   if (netif_is_ovs_master(upper_dev) && vlan_uses_dev(dev))
+   }
+   if (netif_is_ovs_master(upper_dev) && vlan_uses_dev(dev)) {
+   NL_SET_ERR_MSG(extack,
+  

[Bridge] [PATCH v2 net-next 6/7] net: bridge: Pass extack to down to netdev_master_upper_dev_link

[David Ahern]

Pass extack arg to br_add_if. Add messages for a couple of failures
and pass arg to netdev_master_upper_dev_link.

Signed-off-by: David Ahern 
Acked-by: Stephen Hemminger 
---
 net/bridge/br_device.c  |  2 +-
 net/bridge/br_if.c  | 15 +++
 net/bridge/br_ioctl.c   |  2 +-
 net/bridge/br_private.h |  3 ++-
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index cb0131d70ab1..7acb77c9bd65 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -326,7 +326,7 @@ static int br_add_slave(struct net_device *dev, struct 
net_device *slave_dev,
 {
struct net_bridge *br = netdev_priv(dev);
 
-   return br_add_if(br, slave_dev);
+   return br_add_if(br, slave_dev, extack);
 }
 
 static int br_del_slave(struct net_device *dev, struct net_device *slave_dev)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 0a3fd727048d..59a74a414e20 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -480,7 +480,8 @@ netdev_features_t br_features_recompute(struct net_bridge 
*br,
 }
 
 /* called with RTNL */
-int br_add_if(struct net_bridge *br, struct net_device *dev)
+int br_add_if(struct net_bridge *br, struct net_device *dev,
+ struct netlink_ext_ack *extack)
 {
struct net_bridge_port *p;
int err = 0;
@@ -500,16 +501,22 @@ int br_add_if(struct net_bridge *br, struct net_device 
*dev)
return -EINVAL;
 
/* No bridging of bridges */
-   if (dev->netdev_ops->ndo_start_xmit == br_dev_xmit)
+   if (dev->netdev_ops->ndo_start_xmit == br_dev_xmit) {
+   NL_SET_ERR_MSG(extack,
+  "Can not enslave a bridge to a bridge");
return -ELOOP;
+   }
 
/* Device is already being bridged */
if (br_port_exists(dev))
return -EBUSY;
 
/* No bridging devices that dislike that (e.g. wireless) */
-   if (dev->priv_flags & IFF_DONT_BRIDGE)
+   if (dev->priv_flags & IFF_DONT_BRIDGE) {
+   NL_SET_ERR_MSG(extack,
+  "Device does not allow enslaving to a bridge");
return -EOPNOTSUPP;
+   }
 
p = new_nbp(br, dev);
if (IS_ERR(p))
@@ -540,7 +547,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
 
dev->priv_flags |= IFF_BRIDGE_PORT;
 
-   err = netdev_master_upper_dev_link(dev, br->dev, NULL, NULL, NULL);
+   err = netdev_master_upper_dev_link(dev, br->dev, NULL, NULL, extack);
if (err)
goto err5;
 
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index 66cd98772051..8f29103935a3 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -98,7 +98,7 @@ static int add_del_if(struct net_bridge *br, int ifindex, int 
isadd)
return -EINVAL;
 
if (isadd)
-   ret = br_add_if(br, dev);
+   ret = br_add_if(br, dev, NULL);
else
ret = br_del_if(br, dev);
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 020c709a017f..ab4df24f7bba 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -566,7 +566,8 @@ void br_flood(struct net_bridge *br, struct sk_buff *skb,
 void br_port_carrier_check(struct net_bridge_port *p);
 int br_add_bridge(struct net *net, const char *name);
 int br_del_bridge(struct net *net, const char *name);
-int br_add_if(struct net_bridge *br, struct net_device *dev);
+int br_add_if(struct net_bridge *br, struct net_device *dev,
+ struct netlink_ext_ack *extack);
 int br_del_if(struct net_bridge *br, struct net_device *dev);
 int br_min_mtu(const struct net_bridge *br);
 netdev_features_t br_features_recompute(struct net_bridge *br,
-- 
2.1.4

[Bridge] [PATCH v2 net-next 5/7] net: bonding: Add extack messages for some enslave failures

[David Ahern]

A number of bond_enslave errors are logged using the netdev_err API.
Return those messages to userspace via the extack facility.

Signed-off-by: David Ahern 
---
 drivers/net/bonding/bond_main.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index bc92307c2082..172eeeb68152 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1348,12 +1348,14 @@ int bond_enslave(struct net_device *bond_dev, struct 
net_device *slave_dev,
 
/* already in-use? */
if (netdev_is_rx_handler_busy(slave_dev)) {
+   NL_SET_ERR_MSG(extack, "Device is in use and cannot be 
enslaved");
netdev_err(bond_dev,
   "Error: Device is in use and cannot be enslaved\n");
return -EBUSY;
}
 
if (bond_dev == slave_dev) {
+   NL_SET_ERR_MSG(extack, "Cannot enslave bond to itself.");
netdev_err(bond_dev, "cannot enslave bond to itself.\n");
return -EPERM;
}
@@ -1364,6 +1366,7 @@ int bond_enslave(struct net_device *bond_dev, struct 
net_device *slave_dev,
netdev_dbg(bond_dev, "%s is NETIF_F_VLAN_CHALLENGED\n",
   slave_dev->name);
if (vlan_uses_dev(bond_dev)) {
+   NL_SET_ERR_MSG(extack, "Can not enslave VLAN challenged 
device to VLAN enabled bond");
netdev_err(bond_dev, "Error: cannot enslave VLAN 
challenged slave %s on VLAN enabled bond %s\n",
   slave_dev->name, bond_dev->name);
return -EPERM;
@@ -1383,6 +1386,7 @@ int bond_enslave(struct net_device *bond_dev, struct 
net_device *slave_dev,
 * enslaving it; the old ifenslave will not.
 */
if (slave_dev->flags & IFF_UP) {
+   NL_SET_ERR_MSG(extack, "Device can not be enslaved while up");
netdev_err(bond_dev, "%s is up - this may be due to an out of 
date ifenslave\n",
   slave_dev->name);
return -EPERM;
@@ -1423,6 +1427,7 @@ int bond_enslave(struct net_device *bond_dev, struct 
net_device *slave_dev,
 bond_dev);
}
} else if (bond_dev->type != slave_dev->type) {
+   NL_SET_ERR_MSG(extack, "Device type is different from other 
slaves");
netdev_err(bond_dev, "%s ether type (%d) is different from 
other slaves (%d), can not enslave it\n",
   slave_dev->name, slave_dev->type, bond_dev->type);
return -EINVAL;
@@ -1430,6 +1435,7 @@ int bond_enslave(struct net_device *bond_dev, struct 
net_device *slave_dev,
 
if (slave_dev->type == ARPHRD_INFINIBAND &&
BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {
+   NL_SET_ERR_MSG(extack, "Only active-backup mode is supported 
for infiniband slaves");
netdev_warn(bond_dev, "Type (%d) supports only active-backup 
mode\n",
slave_dev->type);
res = -EOPNOTSUPP;
@@ -1445,6 +1451,7 @@ int bond_enslave(struct net_device *bond_dev, struct 
net_device *slave_dev,
bond->params.fail_over_mac = BOND_FOM_ACTIVE;
netdev_warn(bond_dev, "Setting fail_over_mac to 
active for active-backup mode\n");
} else {
+   NL_SET_ERR_MSG(extack, "Slave device does not 
support setting the MAC address, but fail_over_mac is not set to active");
netdev_err(bond_dev, "The slave device 
specified does not support setting the MAC address, but fail_over_mac is not 
set to active\n");
res = -EOPNOTSUPP;
goto err_undo_flags;
-- 
2.1.4

[Bridge] [PATCH v2 net-next 4/7] net: vrf: Add extack messages for enslave errors

[David Ahern]

Signed-off-by: David Ahern 
---
 drivers/net/vrf.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 77d0655a0250..0b54f553228e 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -772,8 +772,11 @@ static int do_vrf_add_slave(struct net_device *dev, struct 
net_device *port_dev,
/* do not allow loopback device to be enslaved to a VRF.
 * The vrf device acts as the loopback for the vrf.
 */
-   if (port_dev == dev_net(dev)->loopback_dev)
+   if (port_dev == dev_net(dev)->loopback_dev) {
+   NL_SET_ERR_MSG(extack,
+  "Can not enslave loopback device to a VRF");
return -EOPNOTSUPP;
+   }
 
port_dev->priv_flags |= IFF_L3MDEV_SLAVE;
ret = netdev_master_upper_dev_link(port_dev, dev, NULL, NULL, extack);
@@ -792,7 +795,13 @@ static int do_vrf_add_slave(struct net_device *dev, struct 
net_device *port_dev,
 static int vrf_add_slave(struct net_device *dev, struct net_device *port_dev,
 struct netlink_ext_ack *extack)
 {
-   if (netif_is_l3_master(port_dev) || netif_is_l3_slave(port_dev))
+   if (netif_is_l3_master(port_dev)) {
+   NL_SET_ERR_MSG(extack,
+  "Can not enslave an L3 master device to a VRF");
+   return -EINVAL;
+   }
+
+   if (netif_is_l3_slave(port_dev))
return -EINVAL;
 
return do_vrf_add_slave(dev, port_dev, extack);
-- 
2.1.4

[Bridge] [PATCH v2 net-next 3/7] net: Add extack to upper device linking

[David Ahern]

Add extack arg to netdev_upper_dev_link and netdev_master_upper_dev_link

Signed-off-by: David Ahern 
---
 drivers/net/bonding/bond_main.c|  7 ---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c |  2 +-
 drivers/net/hyperv/netvsc_drv.c|  2 +-
 drivers/net/ipvlan/ipvlan_main.c   |  2 +-
 drivers/net/macsec.c   |  2 +-
 drivers/net/macvlan.c  |  7 ---
 drivers/net/macvtap.c  |  2 +-
 drivers/net/team/team.c|  2 +-
 drivers/net/usb/qmi_wwan.c |  2 +-
 drivers/net/vrf.c  |  7 ---
 include/linux/if_macvlan.h |  3 ++-
 include/linux/netdevice.h  |  6 --
 net/8021q/vlan.c   |  6 +++---
 net/8021q/vlan.h   |  2 +-
 net/8021q/vlan_netlink.c   |  2 +-
 net/batman-adv/hard-interface.c|  2 +-
 net/bridge/br_if.c |  2 +-
 net/core/dev.c | 15 ++-
 net/openvswitch/vport-netdev.c |  3 ++-
 19 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 78feb94a36db..bc92307c2082 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1217,14 +1217,15 @@ static enum netdev_lag_tx_type bond_lag_tx_type(struct 
bonding *bond)
}
 }
 
-static int bond_master_upper_dev_link(struct bonding *bond, struct slave 
*slave)
+static int bond_master_upper_dev_link(struct bonding *bond, struct slave 
*slave,
+ struct netlink_ext_ack *extack)
 {
struct netdev_lag_upper_info lag_upper_info;
int err;
 
lag_upper_info.tx_type = bond_lag_tx_type(bond);
err = netdev_master_upper_dev_link(slave->dev, bond->dev, slave,
-  _upper_info);
+  _upper_info, extack);
if (err)
return err;
rtmsg_ifinfo(RTM_NEWLINK, slave->dev, IFF_SLAVE, GFP_KERNEL);
@@ -1710,7 +1711,7 @@ int bond_enslave(struct net_device *bond_dev, struct 
net_device *slave_dev,
goto err_detach;
}
 
-   res = bond_master_upper_dev_link(bond, new_slave);
+   res = bond_master_upper_dev_link(bond, new_slave, extack);
if (res) {
netdev_dbg(bond_dev, "Error %d calling 
bond_master_upper_dev_link\n", res);
goto err_unregister;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c 
b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index 98f22551eb45..1af326a60cbb 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -178,7 +178,7 @@ static int rmnet_newlink(struct net *src_net, struct 
net_device *dev,
if (err)
goto err1;
 
-   err = netdev_master_upper_dev_link(dev, real_dev, NULL, NULL);
+   err = netdev_master_upper_dev_link(dev, real_dev, NULL, NULL, extack);
if (err)
goto err2;
 
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f300ae61c6c6..dfb986421ec6 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1748,7 +1748,7 @@ static int netvsc_vf_join(struct net_device *vf_netdev,
goto rx_handler_failed;
}
 
-   ret = netdev_upper_dev_link(vf_netdev, ndev);
+   ret = netdev_upper_dev_link(vf_netdev, ndev, NULL);
if (ret != 0) {
netdev_err(vf_netdev,
   "can not set master device %s (err = %d)\n",
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index c74893c1e620..57c3856bab05 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -584,7 +584,7 @@ int ipvlan_link_new(struct net *src_net, struct net_device 
*dev,
if (err < 0)
goto remove_ida;
 
-   err = netdev_upper_dev_link(phy_dev, dev);
+   err = netdev_upper_dev_link(phy_dev, dev, extack);
if (err) {
goto unregister_netdev;
}
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 98e4deaa3a6a..ccbe4eaffe4d 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3244,7 +3244,7 @@ static int macsec_newlink(struct net *net, struct 
net_device *dev,
   _netdev_addr_lock_key,
   macsec_get_nest_level(dev));
 
-   err = netdev_upper_dev_link(real_dev, dev);
+   err = netdev_upper_dev_link(real_dev, dev, extack);
if (err < 0)
goto unregister;
 

[Bridge] [PATCH v2 net-next 2/7] net: Add extack to ndo_add_slave

[David Ahern]

Pass extack to do_set_master and down to ndo_add_slave

Signed-off-by: David Ahern 
---
 drivers/net/bonding/bond_main.c|  5 +++--
 drivers/net/bonding/bond_options.c |  2 +-
 drivers/net/team/team.c|  3 ++-
 drivers/net/vrf.c  |  3 ++-
 include/linux/netdevice.h  |  3 ++-
 include/net/bonding.h  |  3 ++-
 net/batman-adv/soft-interface.c|  3 ++-
 net/bridge/br_device.c |  3 ++-
 net/core/rtnetlink.c   | 10 ++
 9 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index b19dc033fb36..78feb94a36db 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1328,7 +1328,8 @@ void bond_lower_state_changed(struct slave *slave)
 }
 
 /* enslave device  to bond device  */
-int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
+int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
+struct netlink_ext_ack *extack)
 {
struct bonding *bond = netdev_priv(bond_dev);
const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
@@ -3492,7 +3493,7 @@ static int bond_do_ioctl(struct net_device *bond_dev, 
struct ifreq *ifr, int cmd
switch (cmd) {
case BOND_ENSLAVE_OLD:
case SIOCBONDENSLAVE:
-   res = bond_enslave(bond_dev, slave_dev);
+   res = bond_enslave(bond_dev, slave_dev, NULL);
break;
case BOND_RELEASE_OLD:
case SIOCBONDRELEASE:
diff --git a/drivers/net/bonding/bond_options.c 
b/drivers/net/bonding/bond_options.c
index 5931aa2fe997..8a9b085c2a98 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -1383,7 +1383,7 @@ static int bond_option_slaves_set(struct bonding *bond,
switch (command[0]) {
case '+':
netdev_dbg(bond->dev, "Adding slave %s\n", dev->name);
-   ret = bond_enslave(bond->dev, dev);
+   ret = bond_enslave(bond->dev, dev, NULL);
break;
 
case '-':
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index ae53e899259f..4359d45aa131 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1914,7 +1914,8 @@ static int team_netpoll_setup(struct net_device *dev,
 }
 #endif
 
-static int team_add_slave(struct net_device *dev, struct net_device *port_dev)
+static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
+ struct netlink_ext_ack *extack)
 {
struct team *team = netdev_priv(dev);
int err;
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index cc18b7b11612..4a082ef53533 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -788,7 +788,8 @@ static int do_vrf_add_slave(struct net_device *dev, struct 
net_device *port_dev)
return ret;
 }
 
-static int vrf_add_slave(struct net_device *dev, struct net_device *port_dev)
+static int vrf_add_slave(struct net_device *dev, struct net_device *port_dev,
+struct netlink_ext_ack *extack)
 {
if (netif_is_l3_master(port_dev) || netif_is_l3_slave(port_dev))
return -EINVAL;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 05fcaba4b0d9..368a5064a487 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1246,7 +1246,8 @@ struct net_device_ops {
 u32 flow_id);
 #endif
int (*ndo_add_slave)(struct net_device *dev,
-struct net_device *slave_dev);
+struct net_device *slave_dev,
+struct netlink_ext_ack 
*extack);
int (*ndo_del_slave)(struct net_device *dev,
 struct net_device *slave_dev);
netdev_features_t   (*ndo_fix_features)(struct net_device *dev,
diff --git a/include/net/bonding.h b/include/net/bonding.h
index b2e68657a216..2860cc66c2bb 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -596,7 +596,8 @@ void bond_destroy_sysfs(struct bond_net *net);
 void bond_prepare_sysfs_group(struct bonding *bond);
 int bond_sysfs_slave_add(struct slave *slave);
 void bond_sysfs_slave_del(struct slave *slave);
-int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
+int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
+struct netlink_ext_ack *extack);
 int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
 u32 bond_xmit_hash(struct bonding *bond, struct sk_buff *skb);
 int bond_set_carrier(struct bonding *bond);
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 

[Bridge] [PATCH v2 net-next 0/7] net: Plumb extack error reporting to enslavements

[David Ahern]

Another round of extending extack error reporting, this time for
enslavements through ndo_add_slave and notifiers.

v2
- changed how the messages are added to bonding driver per Jiri's request
- fixed spectrum message for LAG overflow per Ido's comment

David Ahern (7):
  net: Add extack to netdev_notifier_info
  net: Add extack to ndo_add_slave
  net: Add extack to upper device linking
  net: vrf: Add extack messages for enslave errors
  net: bonding: Add extack messages for some enslave failures
  net: bridge: Pass extack to down to netdev_master_upper_dev_link
  mlxsw: spectrum: Add extack messages for enslave failures

 drivers/net/bonding/bond_main.c| 19 +++--
 drivers/net/bonding/bond_options.c |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 47 ---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c |  2 +-
 drivers/net/hyperv/netvsc_drv.c|  2 +-
 drivers/net/ipvlan/ipvlan_main.c   |  2 +-
 drivers/net/macsec.c   |  2 +-
 drivers/net/macvlan.c  |  7 +-
 drivers/net/macvtap.c  |  2 +-
 drivers/net/team/team.c|  5 +-
 drivers/net/usb/qmi_wwan.c |  2 +-
 drivers/net/vrf.c  | 23 --
 include/linux/if_macvlan.h |  3 +-
 include/linux/netdevice.h  | 19 -
 include/net/bonding.h  |  3 +-
 net/8021q/vlan.c   |  6 +-
 net/8021q/vlan.h   |  2 +-
 net/8021q/vlan_netlink.c   |  2 +-
 net/batman-adv/hard-interface.c|  2 +-
 net/batman-adv/soft-interface.c|  3 +-
 net/bridge/br_device.c |  5 +-
 net/bridge/br_if.c | 15 +++-
 net/bridge/br_ioctl.c  |  2 +-
 net/bridge/br_private.h|  3 +-
 net/core/dev.c | 96 +-
 net/core/rtnetlink.c   | 10 ++-
 net/openvswitch/vport-netdev.c |  3 +-
 27 files changed, 192 insertions(+), 97 deletions(-)

-- 
2.1.4

Re: [Bridge] [PATCH net-next v4 1/3] bridge: add new BR_NEIGH_SUPPRESS port flag to suppress arp and nd flood

[David Miller]

From: Roopa Prabhu 
Date: Tue,  3 Oct 2017 22:12:31 -0700

> BR_ARP_PROXY flag but has a few semantic differences to conform

This should be "BR_PROXYARP".

Otherwise this series looks fine to me, but I see there will be
a v5 respin.

Re: [Bridge] [PATCH net-next 5/7] net: bonding: Add extack messages for some enslave failures

[David Ahern]

On 10/4/17 11:04 AM, Jiri Pirko wrote:
> Wed, Oct 04, 2017 at 05:35:46PM CEST, dsah...@gmail.com wrote:
>> On 10/3/17 11:38 PM, Jiri Pirko wrote:
>>> Wed, Oct 04, 2017 at 06:58:52AM CEST, dsah...@gmail.com wrote:
 A number of bond_enslave errors are logged using the netdev_err API.
 Return those messages to userspace via the extack facility.

 Signed-off-by: David Ahern 
 ---
 drivers/net/bonding/bond_main.c | 12 
 1 file changed, 12 insertions(+)

 diff --git a/drivers/net/bonding/bond_main.c 
 b/drivers/net/bonding/bond_main.c
 index bc92307c2082..6688dc9154e0 100644
 --- a/drivers/net/bonding/bond_main.c
 +++ b/drivers/net/bonding/bond_main.c
 @@ -1348,12 +1348,15 @@ int bond_enslave(struct net_device *bond_dev, 
 struct net_device *slave_dev,

/* already in-use? */
if (netdev_is_rx_handler_busy(slave_dev)) {
 +  NL_SET_ERR_MSG(extack,
 + "Device is in use and cannot be enslaved");
>>>
>>> Please don't do this kind of wrapping. Just let the string be on the
>>> same line.
>>>
>>
>> Ok, I will do that for bonding only since it is the existing style.
> 
> I don't believe you need to do this wrap for any code. Just don't wrap.
> General code stype says no wrap for strings :)
> 

I do not break / wrap strings; they need to be searchable. I assumed you
meant this is preferred for bonding:

NL_SET_ERR_MSG(extack, "Device is in use and cannot be enslaved");


over what I have done:

NL_SET_ERR_MSG(extack,
   "Device is in use and cannot be enslaved");

Re: [Bridge] [PATCH net-next 6/7] net: bridge: Pass extack to down to netdev_master_upper_dev_link

[David Ahern]

On 10/4/17 8:13 AM, Stephen Hemminger wrote:
> On Tue,  3 Oct 2017 21:58:53 -0700
> David Ahern  wrote:
> 
>> Pass extack arg to br_add_if. Add messages for a couple of failures
>> and pass arg to netdev_master_upper_dev_link.
>>
>> Signed-off-by: David Ahern 
> 
> This looks good. You might want to pass the netlink_ext_ack down as
> an immutable pointer (const).

Can't mark extack as const. The NL_SET_ERR_MSG sets _msg to the passed
in string.

> 
> Acked-by: Stephen Hemminger 
> 

Re: [Bridge] [PATCH net-next v4 1/3] bridge: add new BR_NEIGH_SUPPRESS port flag to suppress arp and nd flood

[Roopa Prabhu]

On Wed, Oct 4, 2017 at 12:21 AM, Toshiaki Makita
 wrote:
> On 2017/10/04 14:12, Roopa Prabhu wrote:
>> From: Roopa Prabhu 
>>
>> This patch adds a new bridge port flag BR_NEIGH_SUPPRESS to
>> suppress arp and nd flood on bridge ports. It implements
>> rfc7432, section 10.
>> https://tools.ietf.org/html/rfc7432#section-10
>> for ethernet VPN deployments. It is similar to the existing
>> BR_ARP_PROXY flag but has a few semantic differences to conform
>> to EVPN standard. In case of EVPN, it is mainly used to
>> avoid flooding to tunnel ports like vxlan. Unlike the existing
>> flags it suppresses flood of all neigh discovery packets
>> (arp, nd) to tunnel ports.
>>
>> This patch adds netlink and sysfs support to set this bridge port
>> flag.
>>
>> Signed-off-by: Roopa Prabhu 
>> ---
> ...
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index dea88a2..d8c2706 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -138,6 +138,7 @@ static inline size_t br_port_info_size(void)
>>   + nla_total_size(1) /* IFLA_BRPORT_PROXYARP */
>>   + nla_total_size(1) /* IFLA_BRPORT_PROXYARP_WIFI */
>>   + nla_total_size(1) /* IFLA_BRPORT_VLAN_TUNNEL */
>> + + nla_total_size(1) /* IFLA_BRPORT_NEIGH_SUPPRESS */
>>   + nla_total_size(sizeof(struct ifla_bridge_id)) /* 
>> IFLA_BRPORT_ROOT_ID */
>>   + nla_total_size(sizeof(struct ifla_bridge_id)) /* 
>> IFLA_BRPORT_BRIDGE_ID */
>>   + nla_total_size(sizeof(u16))   /* IFLA_BRPORT_DESIGNATED_PORT 
>> */
>> @@ -210,7 +211,9 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>>   nla_put_u8(skb, IFLA_BRPORT_CONFIG_PENDING, p->config_pending) ||
>>   nla_put_u8(skb, IFLA_BRPORT_VLAN_TUNNEL, !!(p->flags &
>>   BR_VLAN_TUNNEL)) ||
>> - nla_put_u16(skb, IFLA_BRPORT_GROUP_FWD_MASK, p->group_fwd_mask))
>> + nla_put_u16(skb, IFLA_BRPORT_GROUP_FWD_MASK, p->group_fwd_mask) ||
>> + nla_put_u8(skb, IFLA_BRPORT_NEIGH_SUPPRESS, !!(p->flags &
>> + BR_NEIGH_SUPPRESS)))
>
> Wouldn't it be better to make the indentation like this?
>
> ... !!(p->flags &
>BR_NEIGH_SUPPRESS)))


not intentional. I think i will actually move the full condition on
the next line.

>
>>   return -EMSGSIZE;
>>
>>   timerval = br_timer_value(>message_age_timer);
>> @@ -692,6 +695,7 @@ static int br_setport(struct net_bridge_port *p, struct 
>> nlattr *tb[])
>>  {
>>   unsigned long old_flags = p->flags;
>>   bool br_vlan_tunnel_old = false;
>> + int neigh_suppress_old = 0;
>>   int err;
>>
>>   err = br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
>> @@ -785,6 +789,12 @@ static int br_setport(struct net_bridge_port *p, struct 
>> nlattr *tb[])
>>   p->group_fwd_mask = fwd_mask;
>>   }
>>
>> + neigh_suppress_old = (p->flags & BR_NEIGH_SUPPRESS);
>> + br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS,
>> +  BR_NEIGH_SUPPRESS);
>> + if (neigh_suppress_old != (p->flags & BR_NEIGH_SUPPRESS))
>> + br_recalculate_neigh_suppress_enabled(p->br);
>> +
>
> You are calling br_recalculate_neigh_suppress_enabled() from within
> br_port_flags_change() immediately after this.
> I think you can just call br_set_port_flag() here.
>
>>   br_port_flags_change(p, old_flags ^ p->flags);
>>   return 0;
>>  }

you are right, i will remove the redundant call to recalc neigh_suppress

Re: [Bridge] [PATCH net-next 7/7] mlxsw: spectrum: Add extack messages for enslave failures

[David Ahern]

On 10/4/17 6:24 AM, Ido Schimmel wrote:
> On Tue, Oct 03, 2017 at 09:58:54PM -0700, David Ahern wrote:
>> mlxsw fails device enslavement for a number of reasons. Use the extack
>> facility to return an error message to the user stating why the enslave
>> is failing.
>>
>> Messages are prefixed with "spectrum" so users know it is a constraint
>> imposed by the hardware driver. For example:
>> $ ip li add br0.11 link br0 type vlan id 11
>> $ ip li set swp11 master br0
>> Error: spectrum: Enslaving a port to a device that already has an upper 
>> device is not supported.
>>
>> Signed-off-by: David Ahern 
> 
> Great stuff, thanks David!
> 
> Reviewed-by: Ido Schimmel 
> Tested-by: Ido Schimmel 
> 
> See one small nit below, but I would like to patch
> mlxsw_sp_netdevice_port_vlan_event() as well, so I can take care of it
> then.

ok. Works for me. I was planning to add more error messages to mlxsw;
happy to concede that to you or someone else. For example a bridge
without vlan filtering:

$ ip li add br1 type bridge
$ ip li set swp20 master br1
RTNETLINK answers: Invalid argument


> 
>> ---
>>  drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 46 
>> --
>>  1 file changed, 36 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c 
>> b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> index 3adf237c951a..a6cc00e4e543 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> @@ -4019,14 +4019,20 @@ static int mlxsw_sp_lag_index_get(struct mlxsw_sp 
>> *mlxsw_sp,
>>  static bool
>>  mlxsw_sp_master_lag_check(struct mlxsw_sp *mlxsw_sp,
>>struct net_device *lag_dev,
>> -  struct netdev_lag_upper_info *lag_upper_info)
>> +  struct netdev_lag_upper_info *lag_upper_info,
>> +  struct netlink_ext_ack *extack)
>>  {
>>  u16 lag_id;
>>  
>> -if (mlxsw_sp_lag_index_get(mlxsw_sp, lag_dev, _id) != 0)
>> +if (mlxsw_sp_lag_index_get(mlxsw_sp, lag_dev, _id) != 0) {
>> +NL_SET_ERR_MSG(extack, "spectrum: Unknown LAG device");
> 
> A more accurate message would be:
> "spectrum: Exceeded number of supported LAG devices"

Right. will update.


> 
>>  return false;
>> -if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
>> +}
>> +if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
>> +NL_SET_ERR_MSG(extack,
>> +   "spectrum: LAG device using unsupported Tx 
>> type");
>>  return false;
>> +}
>>  return true;
>>  }

Re: [Bridge] [PATCH net-next 5/7] net: bonding: Add extack messages for some enslave failures

[David Ahern]

On 10/3/17 11:38 PM, Jiri Pirko wrote:
> Wed, Oct 04, 2017 at 06:58:52AM CEST, dsah...@gmail.com wrote:
>> A number of bond_enslave errors are logged using the netdev_err API.
>> Return those messages to userspace via the extack facility.
>>
>> Signed-off-by: David Ahern 
>> ---
>> drivers/net/bonding/bond_main.c | 12 
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_main.c 
>> b/drivers/net/bonding/bond_main.c
>> index bc92307c2082..6688dc9154e0 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1348,12 +1348,15 @@ int bond_enslave(struct net_device *bond_dev, struct 
>> net_device *slave_dev,
>>
>>  /* already in-use? */
>>  if (netdev_is_rx_handler_busy(slave_dev)) {
>> +NL_SET_ERR_MSG(extack,
>> +   "Device is in use and cannot be enslaved");
> 
> Please don't do this kind of wrapping. Just let the string be on the
> same line.
> 

Ok, I will do that for bonding only since it is the existing style.

Re: [Bridge] [PATCH net-next 6/7] net: bridge: Pass extack to down to netdev_master_upper_dev_link

[Stephen Hemminger]

On Tue,  3 Oct 2017 21:58:53 -0700
David Ahern  wrote:

> Pass extack arg to br_add_if. Add messages for a couple of failures
> and pass arg to netdev_master_upper_dev_link.
> 
> Signed-off-by: David Ahern 

This looks good. You might want to pass the netlink_ext_ack down as
an immutable pointer (const).

Acked-by: Stephen Hemminger 

Re: [Bridge] [PATCH net-next 7/7] mlxsw: spectrum: Add extack messages for enslave failures

[Ido Schimmel]

On Tue, Oct 03, 2017 at 09:58:54PM -0700, David Ahern wrote:
> mlxsw fails device enslavement for a number of reasons. Use the extack
> facility to return an error message to the user stating why the enslave
> is failing.
> 
> Messages are prefixed with "spectrum" so users know it is a constraint
> imposed by the hardware driver. For example:
> $ ip li add br0.11 link br0 type vlan id 11
> $ ip li set swp11 master br0
> Error: spectrum: Enslaving a port to a device that already has an upper 
> device is not supported.
> 
> Signed-off-by: David Ahern 

Great stuff, thanks David!

Reviewed-by: Ido Schimmel 
Tested-by: Ido Schimmel 

See one small nit below, but I would like to patch
mlxsw_sp_netdevice_port_vlan_event() as well, so I can take care of it
then.

> ---
>  drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 46 
> --
>  1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c 
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index 3adf237c951a..a6cc00e4e543 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -4019,14 +4019,20 @@ static int mlxsw_sp_lag_index_get(struct mlxsw_sp 
> *mlxsw_sp,
>  static bool
>  mlxsw_sp_master_lag_check(struct mlxsw_sp *mlxsw_sp,
> struct net_device *lag_dev,
> -   struct netdev_lag_upper_info *lag_upper_info)
> +   struct netdev_lag_upper_info *lag_upper_info,
> +   struct netlink_ext_ack *extack)
>  {
>   u16 lag_id;
>  
> - if (mlxsw_sp_lag_index_get(mlxsw_sp, lag_dev, _id) != 0)
> + if (mlxsw_sp_lag_index_get(mlxsw_sp, lag_dev, _id) != 0) {
> + NL_SET_ERR_MSG(extack, "spectrum: Unknown LAG device");

A more accurate message would be:
"spectrum: Exceeded number of supported LAG devices"

>   return false;
> - if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
> + }
> + if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
> + NL_SET_ERR_MSG(extack,
> +"spectrum: LAG device using unsupported Tx 
> type");
>   return false;
> + }
>   return true;
>  }

Re: [Bridge] [PATCH net-next v4 2/3] bridge: suppress arp pkts on BR_NEIGH_SUPPRESS ports

[Toshiaki Makita]

On 2017/10/04 14:12, Roopa Prabhu wrote:
> From: Roopa Prabhu 
> 
> This patch avoids flooding and proxies arp packets
> for BR_NEIGH_SUPPRESS ports.
> 
> Moves existing br_do_proxy_arp to br_do_proxy_suppress_arp
> to support both proxy arp and neigh suppress.
> 
> Signed-off-by: Roopa Prabhu 
> ---
...
> +static void br_arp_send(struct net_bridge_port *p, int type, int ptype,

type and ptype are always the same so seems unnecessary.

> + __be32 dest_ip, struct net_device *dev,
> + __be32 src_ip, const unsigned char *dest_hw,
> + const unsigned char *src_hw,
> + const unsigned char *target_hw,
> + __be16 vlan_proto, u16 vlan_tci)
> +{
> + struct sk_buff *skb;
> +
> + netdev_dbg(dev, "arp send dev %s dst %pI4 dst_hw %pM src %pI4 src_hw 
> %pM\n",
> +dev->name, _ip, dest_hw, _ip, src_hw);
> +
> + if (!vlan_tci) {
> + arp_send(type, ptype, dest_ip, dev, src_ip,
> +  dest_hw, src_hw, target_hw);

I may be missing something, but wouldn't it send arp reply from the
bridge device, while it should be received to the bridge device, when p
== NULL?

> + return;
> + }
> +
> + skb = arp_create(type, ptype, dest_ip, dev, src_ip,
> +  dest_hw, src_hw, target_hw);
> + if (!skb)
> + return;
> +
> + if (p) {

Why doesn't bridge device consider pvid?

> + struct net_bridge_vlan_group *vg;
> + u16 pvid;
> +
> + vg = nbp_vlan_group_rcu(p);
> + pvid = br_get_pvid(vg);
> + if (pvid == vlan_tci)

Need vlan_tci & VLAN_VID_MASK
Or use skb_vlan_tag_get_id() in caller side.

> + vlan_tci = 0;
> + }
> +
> + if (vlan_tci) {
> + skb = vlan_insert_tag_set_proto(skb, vlan_proto,
> + vlan_tci);

Should be __vlan_hwaccel_put_tag()

> + if (!skb) {
> + net_err_ratelimited("%s: failed to insert VLAN tag\n",
> + __func__);
> + return;
> + }
> + }
> +
> + arp_xmit(skb);
> +}
...
> +void br_do_proxy_suppress_arp(struct sk_buff *skb, struct net_bridge *br,
> +   u16 vid, struct net_bridge_port *p)
> +{
...
> + if (br->neigh_suppress_enabled) {
> + if (p && (p->flags & BR_NEIGH_SUPPRESS))
> + return;
> + if (ipv4_is_zeronet(sip) || sip == tip) {
> + /* prevent flooding to neigh suppress ports */
> + BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
> + return;
> + }
> + }
> +
> + if (parp->ar_op != htons(ARPOP_REQUEST))
> + return;
> +
> + if (vid != 0) {

vid should be 0 if untagged is set on the bridge device?

> + vlandev = __vlan_find_dev_deep_rcu(br->dev, skb->vlan_proto,
> +vid);
> + if (!vlandev)
> + return;
> + }
> +
> + if (br->neigh_suppress_enabled && br_is_local_ip(vlandev, tip)) {
> + /* its our local ip, so don't proxy reply
> +  * and don't forward to neigh suppress ports
> +  */
> + BR_INPUT_SKB_CB(skb)->proxyarp_replied = true;
> + return;
> + }

-- 
Toshiaki Makita

Re: [Bridge] [PATCH net-next v4 1/3] bridge: add new BR_NEIGH_SUPPRESS port flag to suppress arp and nd flood

[Toshiaki Makita]

On 2017/10/04 14:12, Roopa Prabhu wrote:
> From: Roopa Prabhu 
> 
> This patch adds a new bridge port flag BR_NEIGH_SUPPRESS to
> suppress arp and nd flood on bridge ports. It implements
> rfc7432, section 10.
> https://tools.ietf.org/html/rfc7432#section-10
> for ethernet VPN deployments. It is similar to the existing
> BR_ARP_PROXY flag but has a few semantic differences to conform
> to EVPN standard. In case of EVPN, it is mainly used to
> avoid flooding to tunnel ports like vxlan. Unlike the existing
> flags it suppresses flood of all neigh discovery packets
> (arp, nd) to tunnel ports.
> 
> This patch adds netlink and sysfs support to set this bridge port
> flag.
> 
> Signed-off-by: Roopa Prabhu 
> ---
...
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index dea88a2..d8c2706 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -138,6 +138,7 @@ static inline size_t br_port_info_size(void)
>   + nla_total_size(1) /* IFLA_BRPORT_PROXYARP */
>   + nla_total_size(1) /* IFLA_BRPORT_PROXYARP_WIFI */
>   + nla_total_size(1) /* IFLA_BRPORT_VLAN_TUNNEL */
> + + nla_total_size(1) /* IFLA_BRPORT_NEIGH_SUPPRESS */
>   + nla_total_size(sizeof(struct ifla_bridge_id)) /* 
> IFLA_BRPORT_ROOT_ID */
>   + nla_total_size(sizeof(struct ifla_bridge_id)) /* 
> IFLA_BRPORT_BRIDGE_ID */
>   + nla_total_size(sizeof(u16))   /* IFLA_BRPORT_DESIGNATED_PORT 
> */
> @@ -210,7 +211,9 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>   nla_put_u8(skb, IFLA_BRPORT_CONFIG_PENDING, p->config_pending) ||
>   nla_put_u8(skb, IFLA_BRPORT_VLAN_TUNNEL, !!(p->flags &
>   BR_VLAN_TUNNEL)) ||
> - nla_put_u16(skb, IFLA_BRPORT_GROUP_FWD_MASK, p->group_fwd_mask))
> + nla_put_u16(skb, IFLA_BRPORT_GROUP_FWD_MASK, p->group_fwd_mask) ||
> + nla_put_u8(skb, IFLA_BRPORT_NEIGH_SUPPRESS, !!(p->flags &
> + BR_NEIGH_SUPPRESS)))

Wouldn't it be better to make the indentation like this?

... !!(p->flags &
   BR_NEIGH_SUPPRESS)))

>   return -EMSGSIZE;
>  
>   timerval = br_timer_value(>message_age_timer);
> @@ -692,6 +695,7 @@ static int br_setport(struct net_bridge_port *p, struct 
> nlattr *tb[])
>  {
>   unsigned long old_flags = p->flags;
>   bool br_vlan_tunnel_old = false;
> + int neigh_suppress_old = 0;
>   int err;
>  
>   err = br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
> @@ -785,6 +789,12 @@ static int br_setport(struct net_bridge_port *p, struct 
> nlattr *tb[])
>   p->group_fwd_mask = fwd_mask;
>   }
>  
> + neigh_suppress_old = (p->flags & BR_NEIGH_SUPPRESS);
> + br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS,
> +  BR_NEIGH_SUPPRESS);
> + if (neigh_suppress_old != (p->flags & BR_NEIGH_SUPPRESS))
> + br_recalculate_neigh_suppress_enabled(p->br);
> +

You are calling br_recalculate_neigh_suppress_enabled() from within
br_port_flags_change() immediately after this.
I think you can just call br_set_port_flag() here.

>   br_port_flags_change(p, old_flags ^ p->flags);
>   return 0;
>  }

-- 
Toshiaki Makita