Message ID | 50ED6255.7020006@oracle.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
>>> On 09.01.13 at 13:28, jianhai luan <jianhai.luan@oracle.com> wrote: >>> + switch (event) { >>> + case NETDEV_NOTIFY_PEERS: >>> + /* Notify frontend to Send gratuitous ARP */ >>> + xenbus_switch_state(be->dev, XenbusStateInitialised); >>> + xenbus_switch_state(be->dev, ); >> This is the sort of change that clearly isn't acceptable, as I don't >> think you have ways to check _all_ existing frontends for their >> compatibility with this. A connected -> connected transition >> might be acceptable (that was done in the block frontend too, for >> implementing dynamic resize), but will likely need to be >> accompanied by a frontend side patch to handle that (which so >> far should be a no-op). > The latest xen net-frontent driver have handled the condition. State > XenbusStateInitialised will do nothing, > but change to XenbusStateConnected will trigger > netdev_notify_peers(netdev) to send ARP. Did you read my earlier reply carefully? You still only talk about (upstream) Linux netfront, but this is not the only (possible) frontend. You should not invoke state transitions that can - even if only theoretically - blow up frontends. And afaict the only thing you can safely assume frontends ought to tolerate are transitions from Connected to Connected (or more generally from one state to the same one, but the other states aren't useful here, except maybe the Reconfigur* ones). Jan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
于 2013年01月09日 21:44, Jan Beulich 写道: >>>> On 09.01.13 at 13:28, jianhai luan <jianhai.luan@oracle.com> wrote: >>>> + switch (event) { >>>> + case NETDEV_NOTIFY_PEERS: >>>> + /* Notify frontend to Send gratuitous ARP */ >>>> + xenbus_switch_state(be->dev, XenbusStateInitialised); >>>> + xenbus_switch_state(be->dev, ); >>> This is the sort of change that clearly isn't acceptable, as I don't >>> think you have ways to check _all_ existing frontends for their >>> compatibility with this. A connected -> connected transition >>> might be acceptable (that was done in the block frontend too, for >>> implementing dynamic resize), but will likely need to be >>> accompanied by a frontend side patch to handle that (which so >>> far should be a no-op). >> The latest xen net-frontent driver have handled the condition. State >> XenbusStateInitialised will do nothing, >> but change to XenbusStateConnected will trigger >> netdev_notify_peers(netdev) to send ARP. > Did you read my earlier reply carefully? You still only talk about > (upstream) Linux netfront, but this is not the only (possible) > frontend. You should not invoke state transitions that can - > even if only theoretically - blow up frontends. And afaict the I only want to notify xen-netfront. I don't know what is better way? To attainthegoal, i try to modify virtual interrupt, but the way is morecomplicated,modified and working. So, i give up the way. Would you like to give some suggestion about how to notify xen-netfront? > only thing you can safely assume frontends ought to tolerate > are transitions from Connected to Connected (or more > generally from one state to the same one, but the other > states aren't useful here, except maybe the Reconfigur* ones). Sorry for that. At the beginning the patch be applied in kernel 2.6.18 to fixed one issue. Only XenbusStateInitialised and XenbusStateClosed ( Not Reconfigure* one) don't any thing, so i choose the XenbusStateInitialised. Do you suggestion that i choose Reconfigure*? > > Jan > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> On 09.01.13 at 16:37, Jason Luan <luanjianhai@163.com> wrote: > 于 2013年01月09日 21:44, Jan Beulich 写道: >>>>> On 09.01.13 at 13:28, jianhai luan <jianhai.luan@oracle.com> wrote: >>>>> + switch (event) { >>>>> + case NETDEV_NOTIFY_PEERS: >>>>> + /* Notify frontend to Send gratuitous ARP */ >>>>> + xenbus_switch_state(be->dev, XenbusStateInitialised); >>>>> + xenbus_switch_state(be->dev, ); >>>> This is the sort of change that clearly isn't acceptable, as I don't >>>> think you have ways to check _all_ existing frontends for their >>>> compatibility with this. A connected -> connected transition >>>> might be acceptable (that was done in the block frontend too, for >>>> implementing dynamic resize), but will likely need to be >>>> accompanied by a frontend side patch to handle that (which so >>>> far should be a no-op). >>> The latest xen net-frontent driver have handled the condition. State >>> XenbusStateInitialised will do nothing, >>> but change to XenbusStateConnected will trigger >>> netdev_notify_peers(netdev) to send ARP. >> Did you read my earlier reply carefully? You still only talk about >> (upstream) Linux netfront, but this is not the only (possible) >> frontend. You should not invoke state transitions that can - >> even if only theoretically - blow up frontends. And afaict the > I only want to notify xen-netfront. I don't know what is better way? > To attainthegoal, i try to modify virtual interrupt, but the way is > morecomplicated,modified and working. So, i give up the way. > Would you like to give some suggestion about how to notify xen-netfront? >> only thing you can safely assume frontends ought to tolerate >> are transitions from Connected to Connected (or more >> generally from one state to the same one, but the other >> states aren't useful here, except maybe the Reconfigur* ones). > Sorry for that. At the beginning the patch be applied in kernel 2.6.18 to > fixed one issue. Only XenbusStateInitialised and XenbusStateClosed ( Not > Reconfigure* one) don't any thing, so i choose the XenbusStateInitialised. > Do you suggestion that i choose Reconfigure*? I already said that I think that only a Connected->Connected transition is to be considered here. Bringing up Reconfigur* was just to point out that there are other states available that could be used, but you'd first need to make sure that (a) they aren't used for anything else and (b) frontends can reasonably be expected to deal with (ignore) them when not originally aware of them (as opposed to a Connected->Connected transition, which all frontends ought to be able to deal with afaict). Also, when you reply to earlier mails, could you - in order to make the result readable - insert blank lines between the quoted text and your responses, please? Jan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index 410018c..17a3990 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -26,6 +26,7 @@ struct backend_info { struct xenvif *vif; enum xenbus_state frontend_state; struct xenbus_watch hotplug_status_watch; + struct notifier_block vif_notifier; u8 have_hotplug_status_watch:1; }; @@ -34,11 +35,39 @@ static void connect(struct backend_info *); static void backend_create_xenvif(struct backend_info *be); static void unregister_hotplug_status_watch(struct backend_info *be); +#define nb_to_backend(nb) container_of(nb, struct backend_info, vif_notifier) +/** + * When network condition of vif change, notify the frontend. + */ +static int netback_netdev_event(struct notifier_block *this, + unsigned long event, void *ptr) +{ + struct net_device *event_dev = ptr; + struct backend_info *be = nb_to_backend(this); + + pr_debug("event_dev: %s, event: %lx\n", + event_dev ? event_dev->name : "None", event); + + if (!be->vif) + return NOTIFY_DONE; + + switch (event) { + case NETDEV_NOTIFY_PEERS: + /* Notify frontend to Send gratuitous ARP */ + xenbus_switch_state(be->dev, XenbusStateInitialised); + xenbus_switch_state(be->dev, XenbusStateConnected); + break; + } + + return NOTIFY_DONE; +} + static int netback_remove(struct xenbus_device *dev) { struct backend_info *be = dev_get_drvdata(&dev->dev); unregister_hotplug_status_watch(be); + unregister_netdevice_notifier(&be->vif_notifier); if (be->vif) { kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE); xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status"); @@ -129,6 +158,10 @@ static int netback_probe(struct xenbus_device *dev, /* This kicks hotplug scripts, so do it immediately. */ backend_create_xenvif(be); + /* Register Frontend Event Notify */ + be->vif_notifier.notifier_call = netback_netdev_event; + register_netdevice_notifier(&be->vif_notifier); + return 0; abort_transaction: