Skip to contentSkip to conversation

xen-netback notify DomU to send ARP.

PATCH In review jianhai luan 5 participants 44 messages Jan 8, 2013 – Jan 21, 2013 3 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hi,
When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)

              eth1 /
  when eth0 inactive, and eth1 active.
  Guest A --> eth0   bond0 - xenbr0 --VIF(DOMU)
              eth1 /
  Guest A will don't reach to DomU. After Guest A
  send ARP request and DomU respond, Guest A will
  reach DomU. But some more second will be elapsed.
              eth0   bond0 - xenbr0 --VIF(DOMU)

Guest A --> eth1/

If Xen netback watch the network change, will notify DomU by change it own status. So netfront will watch netback's change, and DomU send ARP initiative.

Thanks,
Jason

#1 lore raw
Quoted 4 lines
Quoted 1 line
Quoted 1 line · jianhai luan

When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:

Quoted 11 lines
   ping from Guest A to DomU:
   Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)
               eth1 /
   when eth0 inactive, and eth1 active.
   Guest A --> eth0   bond0 - xenbr0 --VIF(DOMU)
               eth1 /
   Guest A will don't reach to DomU. After Guest A
   send ARP request and DomU respond, Guest A will
   reach DomU. But some more second will be elapsed.
               eth0   bond0 - xenbr0 --VIF(DOMU)
   Guest A --> eth1/

Isn't a change to the availability of the bonds supposed to be transparent to Guest A _and_ DomU? I.e. aren't you trying to fix an eventual problem here in the wrong place?

Quoted 3 lines

If Xen netback watch the network change, will notify DomU by change it own status. So netfront will watch netback's change, and DomU send ARP initiative.

Your patch is, at least according to what I see, completely unusable - line breaks dropped, line order reversed, and (guessing) some UTF-16/UCS-2 characters inserted at the top. Please attach patches as plain ASCII.

Jan

#2 lore raw
Quoted 8 lines · Jan Beulich
Quoted 8 lines
Quoted 1 line
Quoted 1 line · jianhai luan

When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:

   Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)
               eth1 /
   when eth0 inactive, and eth1 active.

How is eth0 failing? Are you unplugging it, un-enslaving it or taking some other sort of administrative action?

Which bonding mode are you using?

Doesn't this state change cause the switch to which eth0 and eth1 are attached to forget the MAC tables associated with the eth0 port, meaning that subsequent traffic will be flooded until it learns that eth1 is the new port?

Quoted 11 lines
Quoted 7 lines
   Guest A --> eth0   bond0 - xenbr0 --VIF(DOMU)
               eth1 /
   Guest A will don't reach to DomU. After Guest A
   send ARP request and DomU respond, Guest A will
   reach DomU. But some more second will be elapsed.
               eth0   bond0 - xenbr0 --VIF(DOMU)
   Guest A --> eth1/

Isn't a change to the availability of the bonds supposed to be transparent to Guest A _and_ DomU? I.e. aren't you trying to fix an eventual problem here in the wrong place?

In non-virtualised bonding the bonding drive can take care of some of this because it knows its own MAC address and can send appropriate gratuitous frames (depends on the bonding mode) to paper over things. In the virtualised case it (most likely) doesn't know VIF(DOMU)s MAC address.

Quoted 10 lines
Quoted 3 lines

If Xen netback watch the network change, will notify DomU by change it own status. So netfront will watch netback's change, and DomU send ARP initiative.

Your patch is, at least according to what I see, completely unusable - line breaks dropped, line order reversed, and (guessing) some UTF-16/UCS-2 characters inserted at the top. Please attach patches as plain ASCII.

From the name it looks to me like it is the vi temp file created while

you have the file open rather than the actual patch file...

Ian.

#3 lore raw
Quoted 11 lines · Ian Campbell
Quoted 8 lines · Jan Beulich
Quoted 8 lines
Quoted 1 line
Quoted 1 line · jianhai luan

When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)
eth1 /
when eth0 inactive, and eth1 active.

How is eth0 failing? Are you unplugging it, un-enslaving it or taking some other sort of administrative action?

In my emulation environment, i unplug it or ifdown the interface, while eth0 maybe wrong in productive environment.

Quoted 2 lines

Which bonding mode are you using?

Bond running in active-backup mode.

Quoted 13 lines

Doesn't this state change cause the switch to which eth0 and eth1 are attached to forget the MAC tables associated with the eth0 port, meaning that subsequent traffic will be flooded until it learns that eth1 is the new port?

Quoted 7 lines
Quoted 7 lines
    Guest A --> eth0   bond0 - xenbr0 --VIF(DOMU)
                eth1 /
    Guest A will don't reach to DomU. After Guest A
    send ARP request and DomU respond, Guest A will
    reach DomU. But some more second will be elapsed.
                eth0   bond0 - xenbr0 --VIF(DOMU)
    Guest A --> eth1/
Quoted 8 lines
Quoted 3 lines

Isn't a change to the availability of the bonds supposed to be transparent to Guest A _and_ DomU? I.e. aren't you trying to fix an eventual problem here in the wrong place?

In non-virtualised bonding the bonding drive can take care of some of this because it knows its own MAC address and can send appropriate gratuitous frames (depends on the bonding mode) to paper over things. In the virtualised case it (most likely) doesn't know VIF(DOMU)s MAC address.

If you have know all ip and mac before modprobe bond, you can
configure the bond argument. But you don't know all ip and mac before start new virtual.
If bond want to know all DomU's ip and mac which pass through, it
must check all skb. it's not good idea.

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

If Xen netback watch the network change, will notify DomU by change it own status. So netfront will watch netback's change, and DomU send ARP initiative.

Quoted 9 lines
Quoted 4 lines

Your patch is, at least according to what I see, completely unusable - line breaks dropped, line order reversed, and (guessing) some UTF-16/UCS-2 characters inserted at the top. Please attach patches as plain ASCII.

From the name it looks to me like it is the vi temp file created while
you have the file open rather than the actual patch file...

Ian.

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#4 lore raw
Quoted 12 lines · jianhai luan
Quoted 10 lines · Ian Campbell
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 2 lines
Quoted 2 lines

On 08.01.13 at 12:57, jianhai luan <[email protected]> wrote:

When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)
eth1 /
when eth0 inactive, and eth1 active.

Quoted 4 lines
Quoted 3 lines

How is eth0 failing? Are you unplugging it, un-enslaving it or taking
some other sort of administrative action?

In my emulation environment, i unplug it or ifdown the interface,

I expect these would behave rather different, since the affect of ifdown looks rather different to an unplug from the PoV of the switch.

Is the ifdown case something which you are trying to solve or just what appeared to be a convenient test case? I'd be less inclined to worry about explict admin actions such as that.

Unplugging the cable should cause:

Quoted 7 lines
Quoted 7 lines

Doesn't this state change cause the switch to which eth0 and eth1 are
attached to forget the MAC tables associated with the eth0 port, meaning
that subsequent traffic will be flooded until it learns that eth1 is the
new port?

Ian

#5 lore raw
Quoted 16 lines · Ian Campbell
Quoted 11 lines · jianhai luan
Quoted 10 lines · Ian Campbell
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 2 lines
Quoted 2 lines

On 08.01.13 at 12:57, jianhai luan <[email protected]> wrote:

When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)
eth1 /
when eth0 inactive, and eth1 active.

Quoted 4 lines
Quoted 3 lines

How is eth0 failing? Are you unplugging it, un-enslaving it or taking
some other sort of administrative action?

In my emulation environment, i unplug it or ifdown the interface,

Quoted 9 lines

I expect these would behave rather different, since the affect of ifdown looks rather different to an unplug from the PoV of the switch.

Is the ifdown case something which you are trying to solve or just what appeared to be a convenient test case? I'd be less inclined to worry about explict admin actions such as that.

Unplugging the cable should cause:

I do above listed thing to let switch active slave only.
I think that we should put attention on the thing which bond switch active slave interface in active-backup mode. In network circumstance, many thing will cause the switch, what do Vif when the event happen?

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Doesn't this state change cause the switch to which eth0 and eth1 are
attached to forget the MAC tables associated with the eth0 port, meaning
that subsequent traffic will be flooded until it learns that eth1 is the
new port?

Quoted 6 lines

Ian

_______________________________________________
Xen-devel mailing list

Thanks,
Jason

Quoted 2 lines

[email protected]
http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#6 lore raw
Quoted 24 lines · Jason Luan
Quoted 23 lines · Ian Campbell
Quoted 11 lines · jianhai luan
Quoted 10 lines · Ian Campbell
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 2 lines
Quoted 2 lines

On 08.01.13 at 12:57, jianhai luan <[email protected]> wrote:

When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)
eth1 /
when eth0 inactive, and eth1 active.

Quoted 4 lines
Quoted 3 lines

How is eth0 failing? Are you unplugging it, un-enslaving it or taking
some other sort of administrative action?

In my emulation environment, i unplug it or ifdown the interface,

I expect these would behave rather different, since the affect of ifdown looks rather different to an unplug from the PoV of the switch.

Is the ifdown case something which you are trying to solve or just what appeared to be a convenient test case? I'd be less inclined to worry about explict admin actions such as that.

Quoted 6 lines
Quoted 2 lines

Unplugging the cable should cause:

I do above listed thing to let switch active slave only.
I think that we should put attention on the thing which bond switch active slave interface in active-backup mode. In network circumstance, many thing will cause the switch, what do Vif when the event happen?

Sorry, I'm having a bit of trouble parsing the above, but are you asking what the VIF should do when the active slave in the bond changes without the previously active slave actually failing?

The issue is that traffic will continue to arrive on the now inactive slave, but will be discarded (the expected behaviour for Active/Passive)?

Is this something which happens in practice? Does the active slave change even while it remains a viable path?

Ian.

#7 lore raw
In reply to Ian Campbell

Sorry,
My attachment is wrong, please check the patch.

Quoted 32 lines · Ian Campbell
Quoted 11 lines · jianhai luan
Quoted 10 lines · Ian Campbell
Quoted 9 lines · Jan Beulich
Quoted 9 lines
Quoted 2 lines
Quoted 2 lines

On 08.01.13 at 12:57, jianhai luan <[email protected]> wrote:

When Xen Dom0's network circumstance changed, DomU
should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)
eth1 /
when eth0 inactive, and eth1 active.

Quoted 4 lines
Quoted 3 lines

How is eth0 failing? Are you unplugging it, un-enslaving it or taking
some other sort of administrative action?

In my emulation environment, i unplug it or ifdown the interface,

I expect these would behave rather different, since the affect of ifdown looks rather different to an unplug from the PoV of the switch.

Is the ifdown case something which you are trying to solve or just what appeared to be a convenient test case? I'd be less inclined to worry about explict admin actions such as that.

Unplugging the cable should cause:

Quoted 7 lines
Quoted 7 lines

Doesn't this state change cause the switch to which eth0 and eth1 are
attached to forget the MAC tables associated with the eth0 port, meaning
that subsequent traffic will be flooded until it learns that eth1 is the
new port?

Quoted 5 lines

Ian

From f1235377724b4363cba27ef8b29fb89e2b36189a Mon Sep 17 00:00:00 2001

From: Jason Luan <[email protected]>
Date: Fri, 28 Dec 2012 15:43:06 +0800
Subject: [PATCH] xen-netback notify DomU to send ARP.

When Xen Dom0's network circumstance changed, DomU should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)

              eth1 /
  when eth0 inactive, and eth1 active.
  Guest A --> eth0   bond0 - xenbr0 --VIF(DOMU)
              eth1 /
  Guest A will don't reach to DomU. After Guest A
  send ARP request and DomU respond, Guest A will
  reach DomU. But some more second will be elapsed.
              eth0   bond0 - xenbr0 --VIF(DOMU)

Guest A --> eth1/

If Xen netback watch the network change, will notify DomU by change it own status. So netfront will watch netback's change, and DomU send ARP initiative.

---
 drivers/net/xen-netback/xenbus.c |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)
drivers/net/xen-netback/xenbus.c +36 -0
index 410018c..ead1a28 100644
@@ -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,42 @@ 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 = (struct net_device *)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)
+		goto out;
+
+	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;
+	default:
+		break;
+	}
+
+out:
+	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 +161,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:
Signature · 1 line
1.7.6.5
#8 lore raw

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 45

Quoted 13 lines
Quoted 1 line
Quoted 1 line · jianhai luan
drivers/net/xen-netback/xenbus.c +8 -0
@@ -34,11 +35,42 @@ 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 = (struct net_device *)ptr;

Pointless cast.

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 58

Quoted 13 lines
drivers/net/xen-netback/xenbus.c +13 -0
+	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)
+		goto out;
+
+	switch (event) {
+	case NETDEV_NOTIFY_PEERS:
+		/* Notify frontend to Send gratuitous ARP */
+		xenbus_switch_state(be->dev, XenbusStateInitialised);
+		xenbus_switch_state(be->dev, XenbusStateConnected);

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).

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 61

Quoted 3 lines
drivers/net/xen-netback/xenbus.c +3 -0
+		break;
+	default:
+		break;

Pointless default case.

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 64

Quoted 3 lines
drivers/net/xen-netback/xenbus.c +3 -0
+	}
+
+out:

I don't think you really need the label (and the goto above) - just put a return there.

Quoted 13 lines
drivers/net/xen-netback/xenbus.c +4 -0
+	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");

drivers/net/xen-netback/xenbus.c static int netback_probe(struct xenbus_device *dev, · line 165

Quoted 5 lines
drivers/net/xen-netback/xenbus.c +2 -0
@@ -129,6 +161,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;

Pointless parentheses.

Jan

Quoted 5 lines
drivers/net/xen-netback/xenbus.c +2 -0
+	register_netdevice_notifier(&be->vif_notifier);
+
 	return 0;
 

abort_transaction:

#9 lore raw
Quoted 27 lines · Jan Beulich

Review static void connect(struct backend_info *); · line 45

Quoted 13 lines
Quoted 1 line
Quoted 1 line · jianhai luan
(quoted diff) +8 -0
@@ -34,11 +35,42 @@ 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 = (struct net_device *)ptr;

Pointless cast.

Quoted 12 lines
(quoted diff) +12 -0
+	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)
+		goto out;
+
+	switch (event) {
+	case NETDEV_NOTIFY_PEERS:
+		/* Notify frontend to Send gratuitous ARP */
+		xenbus_switch_state(be->dev, XenbusStateInitialised);
Quoted 8 lines

Review static void connect(struct backend_info *); · line 58

Quoted 1 line
(quoted diff) +1 -0
+		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.

Quoted 34 lines

Review static void connect(struct backend_info *); · line 61

Quoted 4 lines
(quoted diff) +3 -0
+		break;
+	default:
+		break;

Pointless default case.

Review static void connect(struct backend_info *); · line 64

Quoted 3 lines
(quoted diff) +3 -0
+	}
+
+out:

I don't think you really need the label (and the goto above) - just put a return there.

Quoted 13 lines
(quoted diff) +4 -0
+	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");

Review static int netback_probe(struct xenbus_device *dev, · line 165

Quoted 5 lines
(quoted diff) +2 -0
@@ -129,6 +161,10 @@ static int netback_probe(struct xenbus_device *dev,
 	/* This kicks hotplug scripts, so do it immediately. */
 	backend_create_xenvif(be);
 
+	/*   Event Notify */
+	(be->vif_notifier).notifier_call = netback_netdev_event;

Pointless parentheses.

Jan

Quoted 8 lines
Quoted 5 lines
(quoted diff) +2 -0
+	register_netdevice_notifier(&be->vif_notifier);
+
 	return 0;
 

abort_transaction:

From a64d80cc0c780bee7a8d6e842126cb5f7d17f0d2 Mon Sep 17 00:00:00 2001

From: Jason Luan <[email protected]>
Date: Fri, 28 Dec 2012 15:43:06 +0800
Subject: [PATCH] xen-netback notify DomU to send ARP.

When Xen Dom0's network circumstance changed, DomU should be notified in some special condition. For example the below circumstance:
ping from Guest A to DomU:
Guest A --> eth0 - bond0 - xenbr0 --VIF(DOMU)

              eth1 /
  when eth0 inactive, and eth1 active.
  Guest A --> eth0   bond0 - xenbr0 --VIF(DOMU)
              eth1 /
  Guest A will don't reach to DomU. After Guest A
  send ARP request and DomU respond, Guest A will
  reach DomU. But some more second will be elapsed.
              eth0   bond0 - xenbr0 --VIF(DOMU)

Guest A --> eth1/

If Xen netback watch the network change, will notify DomU by change it own status. So netfront will watch netback's change, and DomU send ARP initiative.

---
 drivers/net/xen-netback/xenbus.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
drivers/net/xen-netback/xenbus.c +33 -0
index 410018c..17a3990 100644
@@ -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:
Signature · 1 line
1.7.6.5
#10 lore raw
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines · jianhai luan
drivers/net/xen-netback/xenbus.c +4 -0
+	switch (event) {
+	case NETDEV_NOTIFY_PEERS:
+		/* Notify frontend to Send gratuitous ARP */
+		xenbus_switch_state(be->dev, XenbusStateInitialised);
Quoted 8 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+		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).

Quoted 4 lines

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

#11 lore raw
Quoted 13 lines · Jan Beulich
Quoted 13 lines
Quoted 13 lines

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *);

Quoted 6 lines · jianhai luan
drivers/net/xen-netback/xenbus.c +5 -0
+	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).

Quoted 8 lines
Quoted 4 lines

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?

Quoted 4 lines

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*?

Quoted 8 lines

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#12 lore raw
Quoted 27 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 13 lines · Jan Beulich
Quoted 13 lines
Quoted 13 lines

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *);

Quoted 6 lines · jianhai luan
drivers/net/xen-netback/xenbus.c +5 -0
+	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).

Quoted 8 lines
Quoted 4 lines

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?

Quoted 8 lines
Quoted 4 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#14 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line · Jason Luan

(re-adding xen-devel)

Quoted 2 lines

I don't know the meaning of Connected->Connected transition, can you give me to detailed explanation?

Just take a look at drivers/block/xen-blkback/blkback.c:xen_vbd_resize().

Jan

A message that is not in the archive (sent off-list or not yet received): [email protected]

#16 lore raw

The patch main to fix the below circumstance:
Bonding run in Active-Backup mode.
Out PC --> switcher
port A --> eth0 --> bond0 --> xenbr0 --> netbackend --> DomU port B -- eth1 /
Or

Out PC (R)--> switcher A --> eth0 --> bond0 --> xenbr0 --> netbackend --> DomU
\ switcher B -- eth1 /

When Switcher Port A (active port) don't reach, or switcher A don't reach, the surroundings will changed to the below circumstance.
Out PC --> switcher
port A -X- eth0 -- bond0 -- xenbr0 -- netbackend -- DomU port B -- eth1 /
Or

Out PC (R)--> switcher A -X- eth0 -- bond0 -- xenbr0 -- netbackend -- DomU
\ switcher B -- eth1 /

So, the former traffic will be unreachable before find the correct path (by sending ARP request).

So, the patch is main to found the bonding change event, and gratutious ARP initialtivly to out PC find the correct path.

the correct path should be the below circumstance:
Out PC --> switcher

            port A -X- eth0 --> bond0 --> xenbr0 --> netbackend --> DomU
            port B --> eth1 /

Or

Out PC (R)--> switcher A -X- eth0 --> bond0 --> xenbr0 --> netbackend --> DomU

            \ switcher B --> eth1 /

Thanks,
Jason

From cd5a33f9137a60f27c2a8c6c77a520d2df356c7a Mon Sep 17 00:00:00 2001
From: Jason Luan <[email protected]>
Date: Fri, 28 Dec 2012 15:43:06 +0800
Subject: [PATCH] xen-netback notify frontend to send gratuitous ARP.

In the real network environment, some cause will lead to Active-Backup mode bonding chose new actived port. After that, the trffic, destinated to DomU by inactived port (former actived port), will be unreachable at now. DomU should send gratutious ARP initialtivly to find the new corrected path.

By netback's Connected->Connected transition, frontend will watch the change, and send gratuitous ARP.

---
 drivers/net/xen-netback/xenbus.c |   68 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)
drivers/net/xen-netback/xenbus.c +68 -0
index 410018c..ebea683 100644
@@ -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,74 @@ 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);
 
+/*
+ * By Connected->Connected transition, netfront will watch the change and
+ * send gratuitous ARP.
+ */
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	struct xenbus_transaction xbt;
+	int err;
+
+	if (xenbus_read_driver_state(dev->nodename) != XenbusStateConnected)
+		return;
+
+again:
+	err = xenbus_transaction_start(&xbt);
+	if (err) {
+		pr_fmt("Error starting transaction");
+		return;
+	}
+
+	err = xenbus_printf(xbt, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");
+		goto abort;
+	}
+
+	err = xenbus_transaction_end(xbt, 0);
+	if (err == -EAGAIN)
+		goto again;
+	if (err)
+		pr_fmt("Error ending transaction");
+
+	return;
+abort:
+	xenbus_transaction_end(xbt, 1);
+}
+
+#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 */
+		notify_front_arping(be->dev);
+		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 +193,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:
Signature · 1 line
1.7.6.5
#17 lore raw

Hi Ian,
Could you give some advice about the patch?

Thanks,
Jason

? 2013?01?10? 15:00, jianhai luan ??:

Quoted 51 lines

The patch main to fix the below circumstance:
Bonding run in Active-Backup mode.
Out PC --> switcher
port A --> eth0 --> bond0 --> xenbr0 --> netbackend --> DomU port B -- eth1 /
Or

Out PC (R)--> switcher A --> eth0 --> bond0 --> xenbr0 --> netbackend --> DomU
\ switcher B -- eth1 /

When Switcher Port A (active port) don't reach, or switcher A don't reach, the surroundings will changed to the below circumstance.
Out PC --> switcher
port A -X- eth0 -- bond0 -- xenbr0 -- netbackend -- DomU port B -- eth1 /
Or

Out PC (R)--> switcher A -X- eth0 -- bond0 -- xenbr0 -- netbackend -- DomU
\ switcher B -- eth1 /

So, the former traffic will be unreachable before find the correct path (by sending ARP request).

So, the patch is main to found the bonding change event, and gratutious ARP initialtivly to out PC find the correct path.

the correct path should be the below circumstance:
Out PC --> switcher

            port A -X- eth0 --> bond0 --> xenbr0 --> netbackend --> DomU
            port B --> eth1 /

Or

Out PC (R)--> switcher A -X- eth0 --> bond0 --> xenbr0 --> netbackend --> DomU

            \ switcher B --> eth1 /

Thanks,
Jason
?????????????
??????????,????????????????????,??????????

0001-xen-netback-notify-frontend-to-send-gratuitous-ARP.patch ??: http://163.fm/DGsSn9l

??: http://163.fm/SKhxM8D

Quoted 4 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#18 lore raw

Hi Ian,
The patch only notify around peer how to reach it when network
circumstance changed.
I don't know what's wrong about the patch (the new feature isn't proper or the patch have other problem)? Why do any body ask the issue?

Thanks,
Jason
? 2013?01?10? 23:20, Jason Luan ??:

Quoted 65 lines

Hi Ian,
Could you give some advice about the patch?

Thanks,
Jason

? 2013?01?10? 15:00, jianhai luan ??:

Quoted 51 lines

The patch main to fix the below circumstance:
Bonding run in Active-Backup mode.
Out PC --> switcher
port A --> eth0 --> bond0 --> xenbr0 --> netbackend --> DomU port B -- eth1 /
Or

Out PC (R)--> switcher A --> eth0 --> bond0 --> xenbr0 --> netbackend --> DomU
\ switcher B -- eth1 /

When Switcher Port A (active port) don't reach, or switcher A don't reach, the surroundings will changed to the below circumstance.
Out PC --> switcher
port A -X- eth0 -- bond0 -- xenbr0 -- netbackend -- DomU port B -- eth1 /
Or

Out PC (R)--> switcher A -X- eth0 -- bond0 -- xenbr0 -- netbackend -- DomU
\ switcher B -- eth1 /

So, the former traffic will be unreachable before find the correct path (by sending ARP request).

So, the patch is main to found the bonding change event, and gratutious ARP initialtivly to out PC find the correct path.

the correct path should be the below circumstance:
Out PC --> switcher

            port A -X- eth0 --> bond0 --> xenbr0 --> netbackend --> DomU
            port B --> eth1 /

Or

Out PC (R)--> switcher A -X- eth0 --> bond0 --> xenbr0 --> netbackend --> DomU

            \ switcher B --> eth1 /

Thanks,
Jason
?????????????
??????????,????????????????????,??????????

0001-xen-netback-notify-frontend-to-send-gratuitous-ARP.patch ??: http://163.fm/DGsSn9l

??: http://163.fm/SKhxM8D

Quoted 4 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Quoted 4 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#19 lore raw
Quoted 5 lines · Jason Luan

Hi Ian,
The patch only notify around peer how to reach it when network
circumstance changed.
I don't know what's wrong about the patch (the new feature isn't proper or the patch have other problem)?

I think the main remaining concern is Jan's regarding what effect this patch will have upon the various existing netfront's out in the wild.

Have you considered the behaviour of various distro kernels, and the pvops and classic-Xen trees?

I don't mind if older kernels fail to ARP on this new transition (they are no worse off than now) but if they crash or something that might be an issue.

Quoted 1 line

Why do any body ask the issue?

What do you mean?

Ian.

#20 lore raw
Quoted 11 lines · Ian Campbell
Quoted 5 lines · Jason Luan

Hi Ian,
The patch only notify around peer how to reach it when network
circumstance changed.
I don't know what's wrong about the patch (the new feature isn't proper or the patch have other problem)?

I think the main remaining concern is Jan's regarding what effect this patch will have upon the various existing netfront's out in the wild.

Have you considered the behaviour of various distro kernels, and the pvops and classic-Xen trees?

Because the patch fix intern bug of oracle bug queue, I have test some netfontends( v2.6.32, v2.6.39 and the latest kernel of Ubuntu 1104 ) on variable netbackend ( v2.6.18, v2.6.32 and v2.6.39) for more time, no other issue be found.

But i don't test classic-xen trees, i will test for whole day at tomorrow.

Quoted 4 lines

I don't mind if older kernels fail to ARP on this new transition (they are no worse off than now) but if they crash or something that might be an issue.

Yes, you are right. But if no other issue be introduced, sending gratuitous ARP initiative will be better. So, Our trouble is which issue will be introduced by the patch at now? I don't found until now. If you have already thought of some bug, would you like to tell me. I will try to fix it.

Quoted 2 lines
Quoted 2 lines

Why do any body ask the issue?

Sorry, It is typo (ask should be ack).

Quoted 9 lines

What do you mean?

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#21 lore raw
Quoted 14 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 11 lines · Ian Campbell
Quoted 5 lines · Jason Luan

Hi Ian,
The patch only notify around peer how to reach it when network
circumstance changed.
I don't know what's wrong about the patch (the new feature isn't proper or the patch have other problem)?

I think the main remaining concern is Jan's regarding what effect this patch will have upon the various existing netfront's out in the wild.

Have you considered the behaviour of various distro kernels, and the pvops and classic-Xen trees?

Quoted 7 lines

Because the patch fix intern bug of oracle bug queue, I have test some netfontends( v2.6.32, v2.6.39 and
the latest kernel of Ubuntu 1104 ) on variable netbackend ( v2.6.18, v2.6.32 and v2.6.39) for more time, no
other issue be found.

But i don't test classic-xen trees, i will test for whole day at tomorrow.

... and Windows and *BSD and Solaris and who knows what else.

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#22 lore raw
Quoted 13 lines · Jan Beulich
Quoted 13 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 11 lines · Ian Campbell
Quoted 5 lines · Jason Luan

Hi Ian,
The patch only notify around peer how to reach it when network
circumstance changed.
I don't know what's wrong about the patch (the new feature isn't proper or the patch have other problem)?

I think the main remaining concern is Jan's regarding what effect this patch will have upon the various existing netfront's out in the wild.

Have you considered the behaviour of various distro kernels, and the pvops and classic-Xen trees?

Quoted 8 lines
Quoted 7 lines

Because the patch fix intern bug of oracle bug queue, I have test some netfontends( v2.6.32, v2.6.39 and
the latest kernel of Ubuntu 1104 ) on variable netbackend ( v2.6.18, v2.6.32 and v2.6.39) for more time, no
other issue be found.

But i don't test classic-xen trees, i will test for whole day at tomorrow.

... and Windows and *BSD and Solaris and who knows what else.

I have tested the issue on Window, Windows don't occur other issue (but Window will don't send ARP) and run good. I don't test the issue on *BSD and Solaris. Do you suggest that i do some test on BSD and Solaris?

Quoted 7 lines

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#23 lore raw
Quoted 21 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 19 lines · Jan Beulich
Quoted 19 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 11 lines · Ian Campbell
Quoted 5 lines · Jason Luan

Hi Ian,
The patch only notify around peer how to reach it when network
circumstance changed.
I don't know what's wrong about the patch (the new feature isn't proper or the patch have other problem)?

I think the main remaining concern is Jan's regarding what effect this patch will have upon the various existing netfront's out in the wild.

Have you considered the behaviour of various distro kernels, and the pvops and classic-Xen trees?

Because the patch fix intern bug of oracle bug queue, I have test some netfontends( v2.6.32, v2.6.39 and
the latest kernel of Ubuntu 1104 ) on variable netbackend ( v2.6.18, v2.6.32 and v2.6.39) for more time, no
other issue be found.

Quoted 7 lines
Quoted 2 lines
Quoted 1 line

But i don't test classic-xen trees, i will test for whole day at tomorrow.

... and Windows and *BSD and Solaris and who knows what else.

I have tested the issue on Window, Windows don't occur other issue (but Window will don't send ARP) and run good.
I don't test the issue on *BSD and Solaris. Do you suggest that i do some test on BSD and Solaris?

My preference would be for you to adjust your patch. Failing that, the next best one is for you to test all existing frontends (including the multiple different ones available for Windows).

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

A message that is not in the archive (sent off-list or not yet received): [email protected]

#25 lore raw

PATCH v2 1/1 xen-netback notify DomU to send ARP.

By Jan and Ian's advice, I have modified the former patch and test some platform, the below is test result:
Linux Pass (Send gratuitous ARP )
Window Pass (don't send ARP, but no other issue occur)
Solaris Pass (don't send ARP, but no other issue occur)
NetBsd Pass (don't send ARP, but no other issue occur)

Thanks,
Jason

Quoted 1 line

From c70af311cc22a1bb04004b03bd5973573360b942 Mon Sep 17 00:00:00 2001

From: Jason Luan <[email protected]>
Date: Fri, 28 Dec 2012 15:43:06 +0800
Subject: [PATCH] xen-netback notify frontend to send gratuitous ARP.

In the real network environment, some cause will lead to Active-Backup mode bonding chose new actived port. After that, the trffic, destinated to DomU by inactived port (former actived port), will be unreachable at now. DomU should send gratutious ARP initialtivly to find the new corrected path.

By netback's Connected->Connected transition, frontend will watch the change, and send gratuitous ARP.

---
 drivers/net/xen-netback/xenbus.c |   51 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)
drivers/net/xen-netback/xenbus.c +51 -0
index 410018c..23e3679 100644
@@ -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,57 @@ 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);
 
+/*
+ * By Connected->Connected transition, netfront will watch the change and
+ * send gratuitous ARP.
+ */
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");
+	}
+
+	return;
+}
+
+#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 */
+		notify_front_arping(be->dev);
+		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 +176,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:
Signature · 1 line
1.7.6.5

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#26 lore raw
Quoted 4 lines · jianhai luan

By Jan and Ian's advice, I have modified the former patch and test some platform, the below is test result:
Linux Pass (Send gratuitous ARP )
Window Pass (don't send ARP, but no other issue occur)

There are several different Windows drivers (from different vendors) and many more different versions. Did you test them all?

Quoted 2 lines

Solaris Pass (don't send ARP, but no other issue occur)
NetBsd Pass (don't send ARP, but no other issue occur)

David

#27 lore raw
Quoted 7 lines · David Vrabel
Quoted 4 lines · jianhai luan

By Jan and Ian's advice, I have modified the former patch and test some platform, the below is test result:
Linux Pass (Send gratuitous ARP )
Window Pass (don't send ARP, but no other issue occur)

There are several different Windows drivers (from different vendors) and many more different versions. Did you test them all?

No, I only test our intern window drivers.
How do i get other window drivers?

Quoted 9 lines
Quoted 3 lines

Solaris Pass (don't send ARP, but no other issue occur)
NetBsd Pass (don't send ARP, but no other issue occur)

David

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#28 lore raw
In reply to jianhai luan

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 51

Quoted 11 lines
Quoted 1 line
Quoted 1 line · jianhai luan
drivers/net/xen-netback/xenbus.c +10 -0
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");

What's this? pr_fmt() alone makes no sense at all, and I'd be pretty surprised if the compiler didn't warn about this construct.

Further, you probably want to say "re-writing" and include the error code in the message. And of course you want a \n at the end.

Finally - no need for the braces ...

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 54

Quoted 3 lines
drivers/net/xen-netback/xenbus.c +3 -0
+	}
+
+	return;

... nor this "return".

Quoted 1 line

+}

#29 lore raw

Which Linux did you test? pvops or something based on the classic-Xen patches?

Quoted 19 lines · Jan Beulich

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 51

Quoted 11 lines
Quoted 1 line
Quoted 1 line · jianhai luan
drivers/net/xen-netback/xenbus.c +10 -0
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");

What's this? pr_fmt() alone makes no sense at all, and I'd be pretty surprised if the compiler didn't warn about this construct.

Further, you probably want to say "re-writing" and include the error code in the message. And of course you want a \n at the end.

Quoted 7 lines

Finally - no need for the braces ...

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 54

Quoted 3 lines
drivers/net/xen-netback/xenbus.c +3 -0
+	}
+
+	return;

... nor this "return".

Agreed on all counts.

Jason, when you resend with these fixes please CC the netdev list and folks listed by ./scripts/get-maintainers.pl for the patch.

Ian.

#30 lore raw
Quoted 18 lines · Ian Campbell

Which Linux did you test? pvops or something based on the classic-Xen patches?

Quoted 14 lines · Jan Beulich

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 51

Quoted 11 lines
Quoted 1 line
Quoted 1 line · jianhai luan
drivers/net/xen-netback/xenbus.c +10 -0
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");

What's this? pr_fmt() alone makes no sense at all, and I'd be pretty surprised if the compiler didn't warn about this construct.

BTW this should probably netif_warn() to get a consistently formatted message.

Ian.

#31 lore raw
In reply to Ian Campbell
Quoted 28 lines · Ian Campbell

Which Linux did you test? pvops or something based on the classic-Xen patches?

Quoted 18 lines · Jan Beulich

Review

Quoted 11 lines
Quoted 1 line
Quoted 1 line · jianhai luan
(quoted diff) +10 -0
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");

What's this? pr_fmt() alone makes no sense at all, and I'd be pretty surprised if the compiler didn't warn about this construct.

Further, you probably want to say "re-writing" and include the error code in the message. And of course you want a \n at the end.

Quoted 6 lines

Finally - no need for the braces ...

Review

Quoted 3 lines
(quoted diff) +3 -0
+	}
+
+	return;

... nor this "return".

Quoted 6 lines

Agreed on all counts.

Jason, when you resend with these fixes please CC the netdev list and folks listed by ./scripts/get-maintainers.pl for the patch.

Ian.

Thank you for your notifying.

Quoted 5 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Thanks,
Jason

Quoted 1 line

From a1acd31b9672d1000549e71a9467b71b1c229d7d Mon Sep 17 00:00:00 2001

From: Jason Luan <[email protected]>
Date: Fri, 28 Dec 2012 15:43:06 +0800
Subject: [PATCH] xen-netback notify frontend to send gratuitous ARP.

In the real network environment, some cause will lead to Active-Backup mode bonding chose new actived port. After that, the trffic, destinated to DomU by inactived port (former actived port), will be unreachable at now. DomU should send gratutious ARP initialtivly to find the new corrected path.

By netback's Connected->Connected transition, frontend will watch the change, and send gratuitous ARP.

---
 drivers/net/xen-netback/xenbus.c |   48 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
drivers/net/xen-netback/xenbus.c +48 -0
index 410018c..844fd86 100644
@@ -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,54 @@ 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);
 
+/*
+ * By Connected->Connected transition, netfront will watch the change and
+ * send gratuitous ARP.
+ */
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err)
+		pr_warn("NetBack: Failure to notify DomU (err=%d)\n", err);
+}
+
+#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 */
+		notify_front_arping(be->dev);
+		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 +173,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:
Signature · 1 line
1.7.9.5

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#32 lore raw
In reply to Ian Campbell
Quoted 2 lines · Ian Campbell

Which Linux did you test? pvops or something based on the classic-Xen patches?

Which Linux, you meaning is which distribution or which version? I have tested Unbreakable Linux on v2.6.18, v2.6.32(need one frontend patch) and v2.6.39, Ubuntu 11.04 on v3.2.0-35.

i misunderstand "pvops or something based on the classic-Xen patches?" , Would you like to give me detailed explanation?

Quoted 31 lines · Jan Beulich
Quoted 18 lines

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 51

Quoted 11 lines
Quoted 1 line
Quoted 1 line · jianhai luan
drivers/net/xen-netback/xenbus.c +10 -0
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");

What's this? pr_fmt() alone makes no sense at all, and I'd be pretty surprised if the compiler didn't warn about this construct.

Further, you probably want to say "re-writing" and include the error code in the message. And of course you want a \n at the end.

Quoted 6 lines

Finally - no need for the braces ...

drivers/net/xen-netback/xenbus.c static void connect(struct backend_info *); · line 54

Quoted 3 lines
drivers/net/xen-netback/xenbus.c +3 -0
+	}
+
+	return;

... nor this "return".

Agreed on all counts.

Jason, when you resend with these fixes please CC the netdev list and folks listed by ./scripts/get-maintainers.pl for the patch.

Quoted 7 lines

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#33 lore raw
Quoted 4 lines · Jason Luan
Quoted 2 lines · Ian Campbell

Which Linux did you test? pvops or something based on the classic-Xen patches?

Quoted 8 lines

Which Linux, you meaning is which distribution or which version? I have tested Unbreakable Linux
on v2.6.18, v2.6.32(need one frontend patch) and v2.6.39, Ubuntu 11.04 on v3.2.0-35.

i misunderstand "pvops or something based on the classic-Xen patches?" , Would you like to give
me detailed explanation?

There are two main "ports" of Xen to Linux, the "classic-Xen" patches are the original XenoLinux port (CONFIG_XEN as a purely compile time option) while pvops is the port which eventually went upstream (dynamic runtime detection of Xen). The frontends differ between the two as well.

Some distros are still based on the classic-Xen patches, and obviously distros released more than a few years ago (i.e. predate pvops) are too. So I was curious if you had tested a kernel of this type as well as the pvops kernel.

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#34 lore raw
Quoted 17 lines · Ian Campbell
Quoted 11 lines · Jason Luan
Quoted 2 lines · Ian Campbell

Which Linux did you test? pvops or something based on the classic-Xen patches?

Which Linux, you meaning is which distribution or which version? I have tested Unbreakable Linux
on v2.6.18, v2.6.32(need one frontend patch) and v2.6.39, Ubuntu 11.04 on v3.2.0-35.

i misunderstand "pvops or something based on the classic-Xen patches?" , Would you like to give
me detailed explanation?

There are two main "ports" of Xen to Linux, the "classic-Xen" patches are the original XenoLinux port (CONFIG_XEN as a purely compile time option) while pvops is the port which eventually went upstream (dynamic runtime detection of Xen). The frontends differ between the two as well.

Quoted 7 lines

Some distros are still based on the classic-Xen patches, and obviously distros released more than a few years ago (i.e. predate pvops) are too. So I was curious if you had tested a kernel of this type as well as the pvops kernel.

Ian.

I see. thank your explain.
Linux which i have tested should be pvops. I can test "Classic-Xen", but where i get?

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#35 lore raw
Quoted 23 lines · Jason Luan
Quoted 22 lines · Ian Campbell
Quoted 11 lines · Jason Luan
Quoted 2 lines · Ian Campbell

Which Linux did you test? pvops or something based on the classic-Xen patches?

Which Linux, you meaning is which distribution or which version? I have tested Unbreakable Linux
on v2.6.18, v2.6.32(need one frontend patch) and v2.6.39, Ubuntu 11.04 on v3.2.0-35.

i misunderstand "pvops or something based on the classic-Xen patches?" , Would you like to give
me detailed explanation?

There are two main "ports" of Xen to Linux, the "classic-Xen" patches are the original XenoLinux port (CONFIG_XEN as a purely compile time option) while pvops is the port which eventually went upstream (dynamic runtime detection of Xen). The frontends differ between the two as well.

Some distros are still based on the classic-Xen patches, and obviously distros released more than a few years ago (i.e. predate pvops) are too. So I was curious if you had tested a kernel of this type as well as the pvops kernel.

Quoted 5 lines
Quoted 2 lines

Ian.

I see. thank your explain.
Linux which i have tested should be pvops. I can test "Classic-Xen", but where i get?

SLES11 and earlier are using the classic-xen patches, as are RHEL5 and earlier (and Centos etc too, maybe even OVM 5?)

http://wiki.xen.org/wiki/Dom0_Kernel_Versions has some mappings from distro to type of kernel as well.

The mercurial repo at http://xenbits.xen.org/hg/linux-2.6.18-xen.hg
contains the source.

Ian.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#37 lore raw
Quoted 8 lines · Jan Beulich
Quoted 3 lines
Quoted 1 line
Quoted 1 line · Jason Luan

Linux which i have tested should be pvops. I can test "Classic-Xen", but where i get?

Your own (Oracle's) 2.6.32 based kernel(s) ought to be such (as being derived from ours, as I was told). And of course you can always build a kernel from http://xenbits.xen.org/hg/linux-2.6.18-xen.hg.

Jan

If that, I have tested the classic-xen on OVM2.2 (kernel v2.6.18), and have tested pvops on Ubuntu 11.04.

Thanks,
Jason

Quoted 5 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#38 lore raw
In reply to Jan Beulich

Hi Jan,
About the patch, what test do i still need to do in the future?
Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

Thanks,
Jason

#39 lore raw
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jason Luan

About the patch, what test do i still need to do in the future? Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

With the revised patch (only doing a Connected->Connected transition now) I don't think you need to do more testing - as said before, I do believe that frontends have to be capable to deal with that (but of course they need to be taught to send an ARP upon that pseudo-transition happening).

Jan

#40 lore raw
Quoted 11 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jason Luan

About the patch, what test do i still need to do in the future? Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

With the revised patch (only doing a Connected->Connected transition now) I don't think you need to do more testing - as said before, I do believe that frontends have to be capable to deal with that (but of course they need to be taught to send an ARP upon that pseudo-transition happening).

Linux frontend have deal with that, and i don't modify other system. What will i do in the future?

Quoted 8 lines

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#41 lore raw
Quoted 8 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 6 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jason Luan

About the patch, what test do i still need to do in the future? Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

Quoted 8 lines
Quoted 5 lines

With the revised patch (only doing a Connected->Connected transition now) I don't think you need to do more testing - as said before, I do believe that frontends have to be capable to deal with that (but of course they need to be taught to send an ARP upon that pseudo-transition happening).

Linux frontend have deal with that, and i don't modify other system. What will i do in the future?

I don't understand what you're asking.

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#42 lore raw
Quoted 8 lines · Jan Beulich
Quoted 8 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 6 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jason Luan

About the patch, what test do i still need to do in the future? Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

Quoted 8 lines
Quoted 7 lines
Quoted 5 lines

With the revised patch (only doing a Connected->Connected transition now) I don't think you need to do more testing - as said before, I do believe that frontends have to be capable to deal with that (but of course they need to be taught to send an ARP upon that pseudo-transition happening).

Linux frontend have deal with that, and i don't modify other system. What will i do in the future?

I don't understand what you're asking.

Sorry for that. I mean that what will be need to do about the patch ?

Quoted 2 lines

Jan

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#43 lore raw
Quoted 15 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 13 lines · Jan Beulich
Quoted 13 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 11 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jason Luan

About the patch, what test do i still need to do in the future? Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

With the revised patch (only doing a Connected->Connected transition now) I don't think you need to do more testing - as said before, I do believe that frontends have to be capable to deal with that (but of course they need to be taught to send an ARP upon that pseudo-transition happening).

Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

Linux frontend have deal with that, and i don't modify other system. What will i do in the future?

I don't understand what you're asking.

Sorry for that. I mean that what will be need to do about the patch ?

The netback one? Nothing more, afaic.

The netfront counterpart I think we're still expecting to see?

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#44 lore raw
Quoted 17 lines · Jan Beulich
Quoted 17 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 15 lines · Jan Beulich
Quoted 15 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 11 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jason Luan

About the patch, what test do i still need to do in the future? Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

With the revised patch (only doing a Connected->Connected transition now) I don't think you need to do more testing - as said before, I do believe that frontends have to be capable to deal with that (but of course they need to be taught to send an ARP upon that pseudo-transition happening).

Linux frontend have deal with that, and i don't modify other system. What will i do in the future?

Quoted 7 lines
Quoted 2 lines
Quoted 1 line

I don't understand what you're asking.

Sorry for that. I mean that what will be need to do about the patch ?

The netback one? Nothing more, afaic.

The netfront counterpart I think we're still expecting to see?

Jan

I See. Thanks your help.

Thanks,
Jason

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#45 lore raw
In reply to Jason Luan
Quoted 14 lines · Jason Luan
Quoted 13 lines · Jan Beulich
Quoted 13 lines
Quoted 1 line
Quoted 1 line · Jason Luan
Quoted 11 lines · Jan Beulich
Quoted 6 lines
Quoted 1 line
Quoted 1 line · Jason Luan

About the patch, what test do i still need to do in the future? Or just be internal patch(after all, OVM just support Linux, window and Solaris ).

Would you give me some advice?

With the revised patch (only doing a Connected->Connected transition now) I don't think you need to do more testing - as said before, I do believe that frontends have to be capable to deal with that (but of course they need to be taught to send an ARP upon that pseudo-transition happening).

Quoted 5 lines
Quoted 3 lines
Quoted 2 lines

Linux frontend have deal with that, and i don't modify other system. What will i do in the future?

I don't understand what you're asking.

Sorry for that. I mean that what will be need to do about the patch ?

Send it to the right lists, please CC the netdev list and folks listed by ./scripts/get-maintainers.pl for the patch.

Ian/

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

#46 lore raw
In reply to Jan Beulich
Quoted 18 lines · Jan Beulich

Review

Quoted 11 lines
Quoted 1 line
Quoted 1 line · jianhai luan
(quoted diff) +10 -0
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) {
+		pr_fmt("Error writing the state");

What's this? pr_fmt() alone makes no sense at all, and I'd be pretty surprised if the compiler didn't warn about this construct.

Further, you probably want to say "re-writing" and include the error code in the message. And of course you want a \n at the end.

Quoted 8 lines

Finally - no need for the braces ...

Review

Quoted 3 lines
(quoted diff) +3 -0
+	}
+
+	return;

... nor this "return".

Quoted 1 line

+}

Sorry for above details, and thank you for notifying.

Quoted 5 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Thanks,
Jason

Quoted 1 line

From 70b6c67228e6d199693a3d3a85a0ab3bf0f2d06a Mon Sep 17 00:00:00 2001

From: Jason Luan <[email protected]>
Date: Fri, 28 Dec 2012 15:43:06 +0800
Subject: [PATCH] xen-netback notify frontend to send gratuitous ARP.

In the real network environment, some cause will lead to Active-Backup mode bonding chose new actived port. After that, the trffic, destinated to DomU by inactived port (former actived port), will be unreachable at now. DomU should send gratutious ARP initialtivly to find the new corrected path.

By netback's Connected->Connected transition, frontend will watch the change, and send gratuitous ARP.

---
 drivers/net/xen-netback/xenbus.c |   48 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
drivers/net/xen-netback/xenbus.c +48 -0
index 410018c..86c05af 100644
@@ -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,54 @@ 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);
 
+/*
+ * By Connected->Connected transition, netfront will watch the change and
+ * send gratuitous ARP.
+ */
+static void notify_front_arping(struct xenbus_device *dev)
+{
+	int err;
+
+	if (dev->state != XenbusStateConnected)
+		return;
+
+	err = xenbus_printf(XBT_NIL, dev->nodename, "state", "%d", dev->state);
+	if (err) 
+		pr_warning("NetBack: Failure to notify DomU (err=%d)\n", err);
+}
+
+#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 */
+		notify_front_arping(be->dev);
+		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 +173,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:
Signature · 1 line
1.7.9.5

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel