Message ID | ac96d2737077b41d1e7cd68164d881faa18f413f.1524395280.git.lucien.xin@gmail.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Series | [net] team: check team dev npinfo when adding a port only | expand |
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
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
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.
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.
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))
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(-)