> > When creating an fcoe interfce, we call fcoe_link_speed_update before we add > the > lports fcoe interface to the fc_hostlist. Since network device events like > NETDEV_CHANGE are only processed if an fcoe interface is found with an > underlying netdev that matches the netdev of the event. Since this processing > in fcoe_device_notification is how link_speed changes get communicated to the > libfc code (via fcoe_link_speed_update), we have a race condition - if a > NETDEV_CHANGE event is sent after the call to fcoe_link_speed_update in > fcoe_netdev_config, but before we add the interface to the fc_hostlist, we > will > loose the event and attributes like /sys/class/fc_host/hostX/speed will not > get > updated properly. > > Fix this by moving the add to the fc_hostlist above the serialized call to > fcoe_netdev_config, ensuring that we catch netdev envents before we make a > direct call to fcoe_link_speed_update. > > Also use this opportunity to clean up access to the fc_hostlist a bit by > creating a fcoe_hostlist_del accessor and replacing the cleanup in fcoe_exit > to > use it properly. > > Tested by myself successfully > > Signed-off-by: Neil Horman <[email protected]> > CC: Robert Love <[email protected]> Looks good to me, good catch for the missing netdev event, minor typo above for 'loose' event => 'lose' event. Bnx2fc may have the similar issue if it has the same netdev notifier and populate the lport list after netdev config, not sure though.
Thanks, Reviewed-by: Yi Zou <[email protected]> > --- > drivers/scsi/fcoe/fcoe.c | 28 ++++++++++++++++++++++++---- > 1 file changed, 24 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c > index 21927f7..2e3478e 100644 > --- a/drivers/scsi/fcoe/fcoe.c > +++ b/drivers/scsi/fcoe/fcoe.c > @@ -87,6 +87,7 @@ static int fcoe_link_ok(struct fc_lport *); > > static struct fc_lport *fcoe_hostlist_lookup(const struct net_device *); > static int fcoe_hostlist_add(const struct fc_lport *); > +static void fcoe_hostlist_del(const struct fc_lport *); > > static int fcoe_device_notification(struct notifier_block *, ulong, void *); > static void fcoe_dev_setup(void); > @@ -1122,6 +1123,9 @@ static struct fc_lport *fcoe_if_create(struct > fcoe_interface *fcoe, > port->min_queue_depth = FCOE_MIN_QUEUE_DEPTH; > INIT_WORK(&port->destroy_work, fcoe_destroy_work); > > + /* Need to add the lport to the hostlist so we catch NETDEV_CHANGE > events */ > + fcoe_hostlist_add(lport); > + > /* configure a fc_lport including the exchange manager */ > rc = fcoe_lport_config(lport); > if (rc) { > @@ -1193,6 +1197,7 @@ static struct fc_lport *fcoe_if_create(struct > fcoe_interface *fcoe, > out_lp_destroy: > fc_exch_mgr_free(lport); > out_host_put: > + fcoe_hostlist_del(lport); > scsi_host_put(lport->host); > out: > return ERR_PTR(rc); > @@ -2316,9 +2321,6 @@ static int _fcoe_create(struct net_device *netdev, > enum fip_state fip_mode, > /* setup DCB priority attributes. */ > fcoe_dcb_create(fcoe); > > - /* add to lports list */ > - fcoe_hostlist_add(lport); > - > /* start FIP Discovery and FLOGI */ > lport->boot_time = jiffies; > fc_fabric_login(lport); > @@ -2560,6 +2562,24 @@ static int fcoe_hostlist_add(const struct fc_lport > *lport) > return 0; > } > > +/** > + * fcoe_hostlist_del() - Remove the FCoE interface identified by a local > + * port to the hostlist > + * @lport: The local port that identifies the FCoE interface to be added > + * > + * Locking: must be called with the RTNL mutex held > + * > + */ > +static void fcoe_hostlist_del(const struct fc_lport *lport) > +{ > + struct fcoe_interface *fcoe; > + struct fcoe_port *port; > + > + port = lport_priv(lport); > + fcoe = port->priv; > + list_del(&fcoe->list); > + return; > +} > > static struct fcoe_transport fcoe_sw_transport = { > .name = {FCOE_TRANSPORT_DEFAULT}, > @@ -2650,9 +2670,9 @@ static void __exit fcoe_exit(void) > /* releases the associated fcoe hosts */ > rtnl_lock(); > list_for_each_entry_safe(fcoe, tmp, &fcoe_hostlist, list) { > - list_del(&fcoe->list); > ctlr = fcoe_to_ctlr(fcoe); > port = lport_priv(ctlr->lp); > + fcoe_hostlist_del(port->lport); > queue_work(fcoe_wq, &port->destroy_work); > } > rtnl_unlock(); > -- > 1.7.11.7 > > _______________________________________________ > fcoe-devel mailing list > [email protected] > http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel _______________________________________________ fcoe-devel mailing list [email protected] http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel
