[PATCH net] team: check team dev npinfo when adding a port only

[PATCH net] team: check team dev npinfo when adding a port only

[Xin Long]

Now when netconsole sets up netpoll on a team dev, __netpoll_setup
will invoke team dev's .ndo_netpoll_setup first, then set
team->dev->npinfo.

However Commit 0fb52a27a04a ("team: cleanup netpoll clode") added
!team->dev->npinfo check in team_port_enable_netpoll(), which is
also invoked by team dev's .ndo_netpoll_setup. It will cause that
port dev npinfo can't be set due to team->dev->npinfo is not yet
set before invoking team dev's .ndo_netpoll_setup.

Team dev only needs to check team->dev->npinfo for setting a port
dev npinfo when adding the port, like before that cleanup.

Fixes: 0fb52a27a04a ("team: cleanup netpoll clode")
Reported-by: João Avelino Bellomo Filho <jbellomo@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 drivers/net/team/team.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index acbe849..f4a0346 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1077,9 +1077,6 @@ static int team_port_enable_netpoll(struct team *team, struct team_port *port)
 	struct netpoll *np;
 	int err;
 
-	if (!team->dev->npinfo)
-		return 0;
-
 	np = kzalloc(sizeof(*np), GFP_KERNEL);
 	if (!np)
 		return -ENOMEM;
@@ -1221,11 +1218,13 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
 		goto err_vids_add;
 	}
 
-	err = team_port_enable_netpoll(team, port);
-	if (err) {
-		netdev_err(dev, "Failed to enable netpoll on device %s\n",
-			   portname);
-		goto err_enable_netpoll;
+	if (team->dev->npinfo) {
+		err = team_port_enable_netpoll(team, port);
+		if (err) {
+			netdev_err(dev, "Failed to enable netpoll on device %s\n",
+				   portname);
+			goto err_enable_netpoll;
+		}
 	}
 
 	if (!(dev->features & NETIF_F_LRO))
-- 
2.1.0

Re: [PATCH net] team: check team dev npinfo when adding a port only

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6108 bytes --]

Hi Xin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Xin-Long/team-check-team-dev-npinfo-when-adding-a-port-only/20180423-114310
config: x86_64-randconfig-x011-201816 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/net/team/team.c: In function 'team_port_add':
>> drivers/net/team/team.c:1221:17: error: 'struct net_device' has no member named 'npinfo'; did you mean 'vlan_info'?
     if (team->dev->npinfo) {
                    ^~~~~~
                    vlan_info

vim +1221 drivers/net/team/team.c

  1136	
  1137	static void __team_port_change_port_added(struct team_port *port, bool linkup);
  1138	static int team_dev_type_check_change(struct net_device *dev,
  1139					      struct net_device *port_dev);
  1140	
  1141	static int team_port_add(struct team *team, struct net_device *port_dev,
  1142				 struct netlink_ext_ack *extack)
  1143	{
  1144		struct net_device *dev = team->dev;
  1145		struct team_port *port;
  1146		char *portname = port_dev->name;
  1147		int err;
  1148	
  1149		if (port_dev->flags & IFF_LOOPBACK) {
  1150			NL_SET_ERR_MSG(extack, "Loopback device can't be added as a team port");
  1151			netdev_err(dev, "Device %s is loopback device. Loopback devices can't be added as a team port\n",
  1152				   portname);
  1153			return -EINVAL;
  1154		}
  1155	
  1156		if (team_port_exists(port_dev)) {
  1157			NL_SET_ERR_MSG(extack, "Device is already a port of a team device");
  1158			netdev_err(dev, "Device %s is already a port "
  1159					"of a team device\n", portname);
  1160			return -EBUSY;
  1161		}
  1162	
  1163		if (port_dev->features & NETIF_F_VLAN_CHALLENGED &&
  1164		    vlan_uses_dev(dev)) {
  1165			NL_SET_ERR_MSG(extack, "Device is VLAN challenged and team device has VLAN set up");
  1166			netdev_err(dev, "Device %s is VLAN challenged and team device has VLAN set up\n",
  1167				   portname);
  1168			return -EPERM;
  1169		}
  1170	
  1171		err = team_dev_type_check_change(dev, port_dev);
  1172		if (err)
  1173			return err;
  1174	
  1175		if (port_dev->flags & IFF_UP) {
  1176			NL_SET_ERR_MSG(extack, "Device is up. Set it down before adding it as a team port");
  1177			netdev_err(dev, "Device %s is up. Set it down before adding it as a team port\n",
  1178				   portname);
  1179			return -EBUSY;
  1180		}
  1181	
  1182		port = kzalloc(sizeof(struct team_port) + team->mode->port_priv_size,
  1183			       GFP_KERNEL);
  1184		if (!port)
  1185			return -ENOMEM;
  1186	
  1187		port->dev = port_dev;
  1188		port->team = team;
  1189		INIT_LIST_HEAD(&port->qom_list);
  1190	
  1191		port->orig.mtu = port_dev->mtu;
  1192		err = dev_set_mtu(port_dev, dev->mtu);
  1193		if (err) {
  1194			netdev_dbg(dev, "Error %d calling dev_set_mtu\n", err);
  1195			goto err_set_mtu;
  1196		}
  1197	
  1198		memcpy(port->orig.dev_addr, port_dev->dev_addr, port_dev->addr_len);
  1199	
  1200		err = team_port_enter(team, port);
  1201		if (err) {
  1202			netdev_err(dev, "Device %s failed to enter team mode\n",
  1203				   portname);
  1204			goto err_port_enter;
  1205		}
  1206	
  1207		err = dev_open(port_dev);
  1208		if (err) {
  1209			netdev_dbg(dev, "Device %s opening failed\n",
  1210				   portname);
  1211			goto err_dev_open;
  1212		}
  1213	
  1214		err = vlan_vids_add_by_dev(port_dev, dev);
  1215		if (err) {
  1216			netdev_err(dev, "Failed to add vlan ids to device %s\n",
  1217					portname);
  1218			goto err_vids_add;
  1219		}
  1220	
> 1221		if (team->dev->npinfo) {
  1222			err = team_port_enable_netpoll(team, port);
  1223			if (err) {
  1224				netdev_err(dev, "Failed to enable netpoll on device %s\n",
  1225					   portname);
  1226				goto err_enable_netpoll;
  1227			}
  1228		}
  1229	
  1230		if (!(dev->features & NETIF_F_LRO))
  1231			dev_disable_lro(port_dev);
  1232	
  1233		err = netdev_rx_handler_register(port_dev, team_handle_frame,
  1234						 port);
  1235		if (err) {
  1236			netdev_err(dev, "Device %s failed to register rx_handler\n",
  1237				   portname);
  1238			goto err_handler_register;
  1239		}
  1240	
  1241		err = team_upper_dev_link(team, port, extack);
  1242		if (err) {
  1243			netdev_err(dev, "Device %s failed to set upper link\n",
  1244				   portname);
  1245			goto err_set_upper_link;
  1246		}
  1247	
  1248		err = __team_option_inst_add_port(team, port);
  1249		if (err) {
  1250			netdev_err(dev, "Device %s failed to add per-port options\n",
  1251				   portname);
  1252			goto err_option_port_add;
  1253		}
  1254	
  1255		netif_addr_lock_bh(dev);
  1256		dev_uc_sync_multiple(port_dev, dev);
  1257		dev_mc_sync_multiple(port_dev, dev);
  1258		netif_addr_unlock_bh(dev);
  1259	
  1260		port->index = -1;
  1261		list_add_tail_rcu(&port->list, &team->port_list);
  1262		team_port_enable(team, port);
  1263		__team_compute_features(team);
  1264		__team_port_change_port_added(port, !!netif_carrier_ok(port_dev));
  1265		__team_options_change_check(team);
  1266	
  1267		netdev_info(dev, "Port device %s added\n", portname);
  1268	
  1269		return 0;
  1270	
  1271	err_option_port_add:
  1272		team_upper_dev_unlink(team, port);
  1273	
  1274	err_set_upper_link:
  1275		netdev_rx_handler_unregister(port_dev);
  1276	
  1277	err_handler_register:
  1278		team_port_disable_netpoll(port);
  1279	
  1280	err_enable_netpoll:
  1281		vlan_vids_del_by_dev(port_dev, dev);
  1282	
  1283	err_vids_add:
  1284		dev_close(port_dev);
  1285	
  1286	err_dev_open:
  1287		team_port_leave(team, port);
  1288		team_port_set_orig_dev_addr(port);
  1289	
  1290	err_port_enter:
  1291		dev_set_mtu(port_dev, port->orig.mtu);
  1292	
  1293	err_set_mtu:
  1294		kfree(port);
  1295	
  1296		return err;
  1297	}
  1298	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30326 bytes --]

Re: [PATCH net] team: check team dev npinfo when adding a port only

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6041 bytes --]

Hi Xin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Xin-Long/team-check-team-dev-npinfo-when-adding-a-port-only/20180423-114310
config: i386-randconfig-x071-201816 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/net/team/team.c: In function 'team_port_add':
>> drivers/net/team/team.c:1221:15: error: 'struct net_device' has no member named 'npinfo'
     if (team->dev->npinfo) {
                  ^~

vim +1221 drivers/net/team/team.c

  1136	
  1137	static void __team_port_change_port_added(struct team_port *port, bool linkup);
  1138	static int team_dev_type_check_change(struct net_device *dev,
  1139					      struct net_device *port_dev);
  1140	
  1141	static int team_port_add(struct team *team, struct net_device *port_dev,
  1142				 struct netlink_ext_ack *extack)
  1143	{
  1144		struct net_device *dev = team->dev;
  1145		struct team_port *port;
  1146		char *portname = port_dev->name;
  1147		int err;
  1148	
  1149		if (port_dev->flags & IFF_LOOPBACK) {
  1150			NL_SET_ERR_MSG(extack, "Loopback device can't be added as a team port");
  1151			netdev_err(dev, "Device %s is loopback device. Loopback devices can't be added as a team port\n",
  1152				   portname);
  1153			return -EINVAL;
  1154		}
  1155	
  1156		if (team_port_exists(port_dev)) {
  1157			NL_SET_ERR_MSG(extack, "Device is already a port of a team device");
  1158			netdev_err(dev, "Device %s is already a port "
  1159					"of a team device\n", portname);
  1160			return -EBUSY;
  1161		}
  1162	
  1163		if (port_dev->features & NETIF_F_VLAN_CHALLENGED &&
  1164		    vlan_uses_dev(dev)) {
  1165			NL_SET_ERR_MSG(extack, "Device is VLAN challenged and team device has VLAN set up");
  1166			netdev_err(dev, "Device %s is VLAN challenged and team device has VLAN set up\n",
  1167				   portname);
  1168			return -EPERM;
  1169		}
  1170	
  1171		err = team_dev_type_check_change(dev, port_dev);
  1172		if (err)
  1173			return err;
  1174	
  1175		if (port_dev->flags & IFF_UP) {
  1176			NL_SET_ERR_MSG(extack, "Device is up. Set it down before adding it as a team port");
  1177			netdev_err(dev, "Device %s is up. Set it down before adding it as a team port\n",
  1178				   portname);
  1179			return -EBUSY;
  1180		}
  1181	
  1182		port = kzalloc(sizeof(struct team_port) + team->mode->port_priv_size,
  1183			       GFP_KERNEL);
  1184		if (!port)
  1185			return -ENOMEM;
  1186	
  1187		port->dev = port_dev;
  1188		port->team = team;
  1189		INIT_LIST_HEAD(&port->qom_list);
  1190	
  1191		port->orig.mtu = port_dev->mtu;
  1192		err = dev_set_mtu(port_dev, dev->mtu);
  1193		if (err) {
  1194			netdev_dbg(dev, "Error %d calling dev_set_mtu\n", err);
  1195			goto err_set_mtu;
  1196		}
  1197	
  1198		memcpy(port->orig.dev_addr, port_dev->dev_addr, port_dev->addr_len);
  1199	
  1200		err = team_port_enter(team, port);
  1201		if (err) {
  1202			netdev_err(dev, "Device %s failed to enter team mode\n",
  1203				   portname);
  1204			goto err_port_enter;
  1205		}
  1206	
  1207		err = dev_open(port_dev);
  1208		if (err) {
  1209			netdev_dbg(dev, "Device %s opening failed\n",
  1210				   portname);
  1211			goto err_dev_open;
  1212		}
  1213	
  1214		err = vlan_vids_add_by_dev(port_dev, dev);
  1215		if (err) {
  1216			netdev_err(dev, "Failed to add vlan ids to device %s\n",
  1217					portname);
  1218			goto err_vids_add;
  1219		}
  1220	
> 1221		if (team->dev->npinfo) {
  1222			err = team_port_enable_netpoll(team, port);
  1223			if (err) {
  1224				netdev_err(dev, "Failed to enable netpoll on device %s\n",
  1225					   portname);
  1226				goto err_enable_netpoll;
  1227			}
  1228		}
  1229	
  1230		if (!(dev->features & NETIF_F_LRO))
  1231			dev_disable_lro(port_dev);
  1232	
  1233		err = netdev_rx_handler_register(port_dev, team_handle_frame,
  1234						 port);
  1235		if (err) {
  1236			netdev_err(dev, "Device %s failed to register rx_handler\n",
  1237				   portname);
  1238			goto err_handler_register;
  1239		}
  1240	
  1241		err = team_upper_dev_link(team, port, extack);
  1242		if (err) {
  1243			netdev_err(dev, "Device %s failed to set upper link\n",
  1244				   portname);
  1245			goto err_set_upper_link;
  1246		}
  1247	
  1248		err = __team_option_inst_add_port(team, port);
  1249		if (err) {
  1250			netdev_err(dev, "Device %s failed to add per-port options\n",
  1251				   portname);
  1252			goto err_option_port_add;
  1253		}
  1254	
  1255		netif_addr_lock_bh(dev);
  1256		dev_uc_sync_multiple(port_dev, dev);
  1257		dev_mc_sync_multiple(port_dev, dev);
  1258		netif_addr_unlock_bh(dev);
  1259	
  1260		port->index = -1;
  1261		list_add_tail_rcu(&port->list, &team->port_list);
  1262		team_port_enable(team, port);
  1263		__team_compute_features(team);
  1264		__team_port_change_port_added(port, !!netif_carrier_ok(port_dev));
  1265		__team_options_change_check(team);
  1266	
  1267		netdev_info(dev, "Port device %s added\n", portname);
  1268	
  1269		return 0;
  1270	
  1271	err_option_port_add:
  1272		team_upper_dev_unlink(team, port);
  1273	
  1274	err_set_upper_link:
  1275		netdev_rx_handler_unregister(port_dev);
  1276	
  1277	err_handler_register:
  1278		team_port_disable_netpoll(port);
  1279	
  1280	err_enable_netpoll:
  1281		vlan_vids_del_by_dev(port_dev, dev);
  1282	
  1283	err_vids_add:
  1284		dev_close(port_dev);
  1285	
  1286	err_dev_open:
  1287		team_port_leave(team, port);
  1288		team_port_set_orig_dev_addr(port);
  1289	
  1290	err_port_enter:
  1291		dev_set_mtu(port_dev, port->orig.mtu);
  1292	
  1293	err_set_mtu:
  1294		kfree(port);
  1295	
  1296		return err;
  1297	}
  1298	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27063 bytes --]

Re: [PATCH net] team: check team dev npinfo when adding a port only

[Xin Long]

On Mon, Apr 23, 2018 at 12:20 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Xin,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on net/master]
>
> url:    https://github.com/0day-ci/linux/commits/Xin-Long/team-check-team-dev-npinfo-when-adding-a-port-only/20180423-114310
> config: i386-randconfig-x071-201816 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>    drivers/net/team/team.c: In function 'team_port_add':
>>> drivers/net/team/team.c:1221:15: error: 'struct net_device' has no member named 'npinfo'
>      if (team->dev->npinfo) {
>                   ^~
Oops, this is different from bonding, we probably should
just revert 0fb52a27a04a ("team: cleanup netpoll clode")
for this fix.

Re: [PATCH net] team: check team dev npinfo when adding a port only

[Xin Long]

On Mon, Apr 23, 2018 at 1:40 PM, Xin Long <lucien.xin@gmail.com> wrote:
> On Mon, Apr 23, 2018 at 12:20 PM, kbuild test robot <lkp@intel.com> wrote:
>> Hi Xin,
>>
>> Thank you for the patch! Yet something to improve:
>>
>> [auto build test ERROR on net/master]
>>
>> url:    https://github.com/0day-ci/linux/commits/Xin-Long/team-check-team-dev-npinfo-when-adding-a-port-only/20180423-114310
>> config: i386-randconfig-x071-201816 (attached as .config)
>> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
>> reproduce:
>>         # save the attached .config to linux build tree
>>         make ARCH=i386
>>
>> All errors (new ones prefixed by >>):
>>
>>    drivers/net/team/team.c: In function 'team_port_add':
>>>> drivers/net/team/team.c:1221:15: error: 'struct net_device' has no member named 'npinfo'
>>      if (team->dev->npinfo) {
>>                   ^~
> Oops, this is different from bonding, we probably should
> just revert 0fb52a27a04a ("team: cleanup netpoll clode")
> for this fix.
or do the same as bridge netpoll does.

+static int team_port_enable_netpoll(struct team *team, struct team_port *port)
+{
+       if (!team->dev->npinfo)
+               return 0;
+
+       return __team_port_enable_netpoll(port);
+}

which looks better.