* [PATCH] hotplug/Linux: update to new ip command syntax. @ 2013-08-16 14:31 Ian Campbell 2013-08-16 19:30 ` Mike 2013-08-19 16:44 ` Ian Jackson 0 siblings, 2 replies; 11+ messages in thread From: Ian Campbell @ 2013-08-16 14:31 UTC (permalink / raw) To: xen-devel; +Cc: Ian Campbell, ian.jackson, Mike From: Mike <debian@good-with-numbers.com> This is Debian bug #705659. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 Mike provided the initial patch to tools/hotplug/Linux/xen-network-common.sh and I extended it to catch some cases in vif-* too. Signed-off-by: Ian Campbell <ijc@hellion.org.uk> Cc: Mike <debian@good-with-numbers.com> --- Mike, could you offer a S-o-b for your bits please, this certifies the change is under the DCO which is described at http://wiki.xen.org/wiki/Submitting_Xen_Patches#Signing_off_a_patch --- tools/hotplug/Linux/vif-bridge | 6 +++--- tools/hotplug/Linux/vif-nat | 2 +- tools/hotplug/Linux/xen-network-common.sh | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge index f489519..55f8b99 100644 --- a/tools/hotplug/Linux/vif-bridge +++ b/tools/hotplug/Linux/vif-bridge @@ -73,7 +73,7 @@ else fi RET=0 -ip link show $bridge 1>/dev/null 2>&1 || RET=1 +ip link show dev $bridge 1>/dev/null 2>&1 || RET=1 if [ "$RET" -eq 1 ] then fatal "Could not find bridge device $bridge" @@ -82,10 +82,10 @@ fi case "$command" in online) setup_virtual_bridge_port "$dev" - mtu="`ip link show $bridge | awk '/mtu/ { print $5 }'`" + mtu="`ip link show dev $bridge | awk '/mtu/ { print $5 }'`" if [ -n "$mtu" ] && [ "$mtu" -gt 0 ] then - ip link set $dev mtu $mtu || : + ip link set dev $dev mtu $mtu || : fi add_to_bridge "$bridge" "$dev" ;; diff --git a/tools/hotplug/Linux/vif-nat b/tools/hotplug/Linux/vif-nat index 8d29fb6..0b900d5 100644 --- a/tools/hotplug/Linux/vif-nat +++ b/tools/hotplug/Linux/vif-nat @@ -170,7 +170,7 @@ case "$command" in exit 0 fi - do_or_die ip link set "${dev}" up arp on + do_or_die ip link set dev "${dev}" up arp on do_or_die ip addr add "$router_ip" dev "${dev}" do_or_die ip route add "$vif_ip" dev "${dev}" src "$router_ip" echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh index 8cff156..db030d8 100644 --- a/tools/hotplug/Linux/xen-network-common.sh +++ b/tools/hotplug/Linux/xen-network-common.sh @@ -85,18 +85,18 @@ _setup_bridge_port() { local virtual="$2" # take interface down ... - ip link set ${dev} down + ip link set dev ${dev} down if [ $virtual -ne 0 ] ; then # Initialise a dummy MAC address. We choose the numerically # largest non-broadcast address to prevent the address getting # stolen by an Ethernet bridge for STP purposes. # (FE:FF:FF:FF:FF:FF) - ip link set ${dev} address fe:ff:ff:ff:ff:ff || true + ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true fi # ... and configure it - ip addr flush ${dev} + ip address flush dev ${dev} } setup_physical_bridge_port() { @@ -125,10 +125,10 @@ add_to_bridge () { # Don't add $dev to $bridge if it's already on a bridge. if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then - ip link set ${dev} up || true + ip link set dev ${dev} up || true return fi brctl addif ${bridge} ${dev} - ip link set ${dev} up + ip link set dev ${dev} up } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-08-16 14:31 [PATCH] hotplug/Linux: update to new ip command syntax Ian Campbell @ 2013-08-16 19:30 ` Mike 2013-08-19 16:44 ` Ian Jackson 1 sibling, 0 replies; 11+ messages in thread From: Mike @ 2013-08-16 19:30 UTC (permalink / raw) To: Ian Campbell; +Cc: ian.jackson, xen-devel Signed-off-by: Ian Campbell <ijc@hellion.org.uk> Signed-off-by: Mike <debian@good-with-numbers.com> --- tools/hotplug/Linux/vif-bridge | 6 +++--- tools/hotplug/Linux/vif-nat | 2 +- tools/hotplug/Linux/xen-network-common.sh | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge index f489519..55f8b99 100644 --- a/tools/hotplug/Linux/vif-bridge +++ b/tools/hotplug/Linux/vif-bridge @@ -73,7 +73,7 @@ else fi RET=0 -ip link show $bridge 1>/dev/null 2>&1 || RET=1 +ip link show dev $bridge 1>/dev/null 2>&1 || RET=1 if [ "$RET" -eq 1 ] then fatal "Could not find bridge device $bridge" @@ -82,10 +82,10 @@ fi case "$command" in online) setup_virtual_bridge_port "$dev" - mtu="`ip link show $bridge | awk '/mtu/ { print $5 }'`" + mtu="`ip link show dev $bridge | awk '/mtu/ { print $5 }'`" if [ -n "$mtu" ] && [ "$mtu" -gt 0 ] then - ip link set $dev mtu $mtu || : + ip link set dev $dev mtu $mtu || : fi add_to_bridge "$bridge" "$dev" ;; diff --git a/tools/hotplug/Linux/vif-nat b/tools/hotplug/Linux/vif-nat index 8d29fb6..0b900d5 100644 --- a/tools/hotplug/Linux/vif-nat +++ b/tools/hotplug/Linux/vif-nat @@ -170,7 +170,7 @@ case "$command" in exit 0 fi - do_or_die ip link set "${dev}" up arp on + do_or_die ip link set dev "${dev}" up arp on do_or_die ip addr add "$router_ip" dev "${dev}" do_or_die ip route add "$vif_ip" dev "${dev}" src "$router_ip" echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh index 8cff156..db030d8 100644 --- a/tools/hotplug/Linux/xen-network-common.sh +++ b/tools/hotplug/Linux/xen-network-common.sh @@ -85,18 +85,18 @@ _setup_bridge_port() { local virtual="$2" # take interface down ... - ip link set ${dev} down + ip link set dev ${dev} down if [ $virtual -ne 0 ] ; then # Initialise a dummy MAC address. We choose the numerically # largest non-broadcast address to prevent the address getting # stolen by an Ethernet bridge for STP purposes. # (FE:FF:FF:FF:FF:FF) - ip link set ${dev} address fe:ff:ff:ff:ff:ff || true + ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true fi # ... and configure it - ip addr flush ${dev} + ip address flush dev ${dev} } setup_physical_bridge_port() { @@ -125,10 +125,10 @@ add_to_bridge () { # Don't add $dev to $bridge if it's already on a bridge. if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then - ip link set ${dev} up || true + ip link set dev ${dev} up || true return fi brctl addif ${bridge} ${dev} - ip link set ${dev} up + ip link set dev ${dev} up } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-08-16 14:31 [PATCH] hotplug/Linux: update to new ip command syntax Ian Campbell 2013-08-16 19:30 ` Mike @ 2013-08-19 16:44 ` Ian Jackson 2013-08-19 17:01 ` Ian Campbell 1 sibling, 1 reply; 11+ messages in thread From: Ian Jackson @ 2013-08-19 16:44 UTC (permalink / raw) To: Ian Campbell; +Cc: Mike, xen-devel Ian Campbell writes ("[PATCH] hotplug/Linux: update to new ip command syntax."): > From: Mike <debian@good-with-numbers.com> > > This is Debian bug #705659. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 > > Mike provided the initial patch to tools/hotplug/Linux/xen-network-common.sh > and I extended it to catch some cases in vif-* too. > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > Cc: Mike <debian@good-with-numbers.com> Do we know whether this new syntax is backwards-compatible ? Ian. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-08-19 16:44 ` Ian Jackson @ 2013-08-19 17:01 ` Ian Campbell 2013-11-19 12:23 ` Ian Campbell 0 siblings, 1 reply; 11+ messages in thread From: Ian Campbell @ 2013-08-19 17:01 UTC (permalink / raw) To: Ian Jackson; +Cc: Mike, xen-devel On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote: > Ian Campbell writes ("[PATCH] hotplug/Linux: update to new ip command syntax."): > > From: Mike <debian@good-with-numbers.com> > > > > This is Debian bug #705659. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 > > > > Mike provided the initial patch to tools/hotplug/Linux/xen-network-common.sh > > and I extended it to catch some cases in vif-* too. > > > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > Cc: Mike <debian@good-with-numbers.com> > > Do we know whether this new syntax is backwards-compatible ? Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" in the synopsis by virtue of not defining DEVICE anywhere but it does show the use of dev later on and the tool accepts it. Likewise for "ip link set". Given this, as Mike says, makes it impossible to name a device "dev" it seems likely to be a documentation bug. "ip addr flush" is correctly documented as needing the dev (by saying "dev STRING" and not "DEVICE" , I didn't try without but I assume it works since our scripts do that right now. I wasn't easily able to come up with an older machine. Ian. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-08-19 17:01 ` Ian Campbell @ 2013-11-19 12:23 ` Ian Campbell 2013-11-20 14:37 ` Ian Jackson 0 siblings, 1 reply; 11+ messages in thread From: Ian Campbell @ 2013-11-19 12:23 UTC (permalink / raw) To: Ian Jackson; +Cc: xen-devel, Mike On Mon, 2013-08-19 at 18:01 +0100, Ian Campbell wrote: > On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote: > > Ian Campbell writes ("[PATCH] hotplug/Linux: update to new ip command syntax."): > > > From: Mike <debian@good-with-numbers.com> > > > > > > This is Debian bug #705659. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 > > > > > > Mike provided the initial patch to tools/hotplug/Linux/xen-network-common.sh > > > and I extended it to catch some cases in vif-* too. > > > > > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > > Cc: Mike <debian@good-with-numbers.com> > > > > Do we know whether this new syntax is backwards-compatible ? > > Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" in > the synopsis by virtue of not defining DEVICE anywhere but it does show > the use of dev later on and the tool accepts it. Likewise for "ip link > set". Given this, as Mike says, makes it impossible to name a device > "dev" it seems likely to be a documentation bug. > > "ip addr flush" is correctly documented as needing the dev (by saying > "dev STRING" and not "DEVICE" , I didn't try without but I assume it > works since our scripts do that right now. > > I wasn't easily able to come up with an older machine. Ian, was that a satisfactory explanation? Ian. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-11-19 12:23 ` Ian Campbell @ 2013-11-20 14:37 ` Ian Jackson 2013-11-20 14:53 ` Ian Campbell 0 siblings, 1 reply; 11+ messages in thread From: Ian Jackson @ 2013-11-20 14:37 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel, Mike Ian Campbell writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax."): > On Mon, 2013-08-19 at 18:01 +0100, Ian Campbell wrote: > > On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote: > > > Do we know whether this new syntax is backwards-compatible ? > > > > Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" in > > the synopsis by virtue of not defining DEVICE anywhere but it does show > > the use of dev later on and the tool accepts it. Likewise for "ip link > > set". Given this, as Mike says, makes it impossible to name a device > > "dev" it seems likely to be a documentation bug. > > > > "ip addr flush" is correctly documented as needing the dev (by saying > > "dev STRING" and not "DEVICE" , I didn't try without but I assume it > > works since our scripts do that right now. > > > > I wasn't easily able to come up with an older machine. > > Ian, was that a satisfactory explanation? Yes, it was. Sorry. Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Ian. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-11-20 14:37 ` Ian Jackson @ 2013-11-20 14:53 ` Ian Campbell 2013-11-20 18:50 ` George Dunlap 0 siblings, 1 reply; 11+ messages in thread From: Ian Campbell @ 2013-11-20 14:53 UTC (permalink / raw) To: Ian Jackson; +Cc: George Dunlap, xen-devel, Mike On Wed, 2013-11-20 at 14:37 +0000, Ian Jackson wrote: > Ian Campbell writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax."): > > On Mon, 2013-08-19 at 18:01 +0100, Ian Campbell wrote: > > > On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote: > > > > Do we know whether this new syntax is backwards-compatible ? > > > > > > Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" in > > > the synopsis by virtue of not defining DEVICE anywhere but it does show > > > the use of dev later on and the tool accepts it. Likewise for "ip link > > > set". Given this, as Mike says, makes it impossible to name a device > > > "dev" it seems likely to be a documentation bug. > > > > > > "ip addr flush" is correctly documented as needing the dev (by saying > > > "dev STRING" and not "DEVICE" , I didn't try without but I assume it > > > works since our scripts do that right now. > > > > > > I wasn't easily able to come up with an older machine. > > > > Ian, was that a satisfactory explanation? > > Yes, it was. Sorry. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Thanks. Now, WRT 4.4... With reference to bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 the patch allows the creation of a vif named "dev". Which although I'm sure useful it is easily worked around. But on the other hand we are early in the freeze and this could easily be considered a bug fix. At this stage I'm leaning towards say lets take it. Ian. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-11-20 14:53 ` Ian Campbell @ 2013-11-20 18:50 ` George Dunlap 2013-11-21 10:01 ` Ian Campbell 0 siblings, 1 reply; 11+ messages in thread From: George Dunlap @ 2013-11-20 18:50 UTC (permalink / raw) To: Ian Campbell; +Cc: Mike, Ian Jackson, xen-devel On Wed, Nov 20, 2013 at 2:53 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Wed, 2013-11-20 at 14:37 +0000, Ian Jackson wrote: >> Ian Campbell writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax."): >> > On Mon, 2013-08-19 at 18:01 +0100, Ian Campbell wrote: >> > > On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote: >> > > > Do we know whether this new syntax is backwards-compatible ? >> > > >> > > Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" in >> > > the synopsis by virtue of not defining DEVICE anywhere but it does show >> > > the use of dev later on and the tool accepts it. Likewise for "ip link >> > > set". Given this, as Mike says, makes it impossible to name a device >> > > "dev" it seems likely to be a documentation bug. >> > > >> > > "ip addr flush" is correctly documented as needing the dev (by saying >> > > "dev STRING" and not "DEVICE" , I didn't try without but I assume it >> > > works since our scripts do that right now. >> > > >> > > I wasn't easily able to come up with an older machine. >> > >> > Ian, was that a satisfactory explanation? >> >> Yes, it was. Sorry. >> >> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > Thanks. > > Now, WRT 4.4... > > With reference to bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 the > patch allows the creation of a vif named "dev". Which although I'm sure > useful it is easily worked around. But on the other hand we are early in > the freeze and this could easily be considered a bug fix. > > At this stage I'm leaning towards say lets take it. Should I say 'no' once just to let people know who's in charge? :-) I might be inclined to do so actually, just on principle; except that the best way to get this actually tested will be the upcoming test days, when (hopefully) it will be tested on all the major distros. If we put it off until 4.5, I doubt it will get much more testing than it would right now. One thing I'm not happy with the patch though -- it doesn't have a proper description of the problem and what the patch is actually doing in the changeset itself. Links to supplemental information are fine, I think, but we have to assume that 1) people will be browsing the source code offline, and 2) links will disappear; so the critical information needs to be included. -George ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-11-20 18:50 ` George Dunlap @ 2013-11-21 10:01 ` Ian Campbell 2013-11-21 15:08 ` George Dunlap 0 siblings, 1 reply; 11+ messages in thread From: Ian Campbell @ 2013-11-21 10:01 UTC (permalink / raw) To: George Dunlap; +Cc: Mike, Ian Jackson, xen-devel On Wed, 2013-11-20 at 18:50 +0000, George Dunlap wrote: > On Wed, Nov 20, 2013 at 2:53 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Wed, 2013-11-20 at 14:37 +0000, Ian Jackson wrote: > >> Ian Campbell writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax."): > >> > On Mon, 2013-08-19 at 18:01 +0100, Ian Campbell wrote: > >> > > On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote: > >> > > > Do we know whether this new syntax is backwards-compatible ? > >> > > > >> > > Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" in > >> > > the synopsis by virtue of not defining DEVICE anywhere but it does show > >> > > the use of dev later on and the tool accepts it. Likewise for "ip link > >> > > set". Given this, as Mike says, makes it impossible to name a device > >> > > "dev" it seems likely to be a documentation bug. > >> > > > >> > > "ip addr flush" is correctly documented as needing the dev (by saying > >> > > "dev STRING" and not "DEVICE" , I didn't try without but I assume it > >> > > works since our scripts do that right now. > >> > > > >> > > I wasn't easily able to come up with an older machine. > >> > > >> > Ian, was that a satisfactory explanation? > >> > >> Yes, it was. Sorry. > >> > >> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > > > Thanks. > > > > Now, WRT 4.4... > > > > With reference to bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 the > > patch allows the creation of a vif named "dev". Which although I'm sure > > useful it is easily worked around. But on the other hand we are early in > > the freeze and this could easily be considered a bug fix. > > > > At this stage I'm leaning towards say lets take it. > > Should I say 'no' once just to let people know who's in charge? :-) > > I might be inclined to do so actually, just on principle; except that > the best way to get this actually tested will be the upcoming test > days, when (hopefully) it will be tested on all the major distros. If > we put it off until 4.5, I doubt it will get much more testing than it > would right now. > > One thing I'm not happy with the patch though -- it doesn't have a > proper description of the problem and what the patch is actually doing > in the changeset itself. Links to supplemental information are fine, > I think, but we have to assume that 1) people will be browsing the > source code offline, and 2) links will disappear; so the critical > information needs to be included. Ack. 8>---------------------- >From aed6743365d17c58561481ce1d74613f9a731746 Mon Sep 17 00:00:00 2001 From: Mike <debian@good-with-numbers.com> Date: Fri, 16 Aug 2013 15:31:43 +0100 Subject: [PATCH] hotplug/Linux: update to new ip command syntax. The current usage prevents naming a vif "dev". Although the current syntax is documented in Squeeze's ip(8) it appears that this was a documentation bug. Newer versions of the man page describe the new syntax used here and Squeeze's implementation accepts it as well. This is Debian bug #705659. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 Mike provided the initial patch to tools/hotplug/Linux/xen-network-common.sh and Ian extended it to catch some cases in vif-* too. Signed-off-by: Ian Campbell <ijc@hellion.org.uk> Signed-off-by: Mike <debian@good-with-numbers.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> --- v2: Updated commit message. --- tools/hotplug/Linux/vif-bridge | 2 +- tools/hotplug/Linux/vif-nat | 2 +- tools/hotplug/Linux/xen-network-common.sh | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge index 678262d..b7dcbd6 100644 --- a/tools/hotplug/Linux/vif-bridge +++ b/tools/hotplug/Linux/vif-bridge @@ -72,7 +72,7 @@ else fi RET=0 -ip link show $bridge 1>/dev/null 2>&1 || RET=1 +ip link show dev $bridge 1>/dev/null 2>&1 || RET=1 if [ "$RET" -eq 1 ] then fatal "Could not find bridge device $bridge" diff --git a/tools/hotplug/Linux/vif-nat b/tools/hotplug/Linux/vif-nat index 8d29fb6..0b900d5 100644 --- a/tools/hotplug/Linux/vif-nat +++ b/tools/hotplug/Linux/vif-nat @@ -170,7 +170,7 @@ case "$command" in exit 0 fi - do_or_die ip link set "${dev}" up arp on + do_or_die ip link set dev "${dev}" up arp on do_or_die ip addr add "$router_ip" dev "${dev}" do_or_die ip route add "$vif_ip" dev "${dev}" src "$router_ip" echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh index 50b8711..3c63c55 100644 --- a/tools/hotplug/Linux/xen-network-common.sh +++ b/tools/hotplug/Linux/xen-network-common.sh @@ -85,18 +85,18 @@ _setup_bridge_port() { local virtual="$2" # take interface down ... - ip link set ${dev} down + ip link set dev ${dev} down if [ $virtual -ne 0 ] ; then # Initialise a dummy MAC address. We choose the numerically # largest non-broadcast address to prevent the address getting # stolen by an Ethernet bridge for STP purposes. # (FE:FF:FF:FF:FF:FF) - ip link set ${dev} address fe:ff:ff:ff:ff:ff || true + ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true fi # ... and configure it - ip addr flush ${dev} + ip address flush dev ${dev} } setup_physical_bridge_port() { @@ -125,20 +125,20 @@ add_to_bridge () { # Don't add $dev to $bridge if it's already on a bridge. if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then - ip link set ${dev} up || true + ip link set dev ${dev} up || true return fi brctl addif ${bridge} ${dev} - ip link set ${dev} up + ip link set dev ${dev} up } # Usage: set_mtu bridge dev set_mtu () { local bridge=$1 local dev=$2 - mtu="`ip link show ${bridge}| awk '/mtu/ { print $5 }'`" + mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`" if [ -n "$mtu" ] && [ "$mtu" -gt 0 ] then - ip link set ${dev} mtu $mtu || : + ip link set dev ${dev} mtu $mtu || : fi } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-11-21 10:01 ` Ian Campbell @ 2013-11-21 15:08 ` George Dunlap 2013-11-21 18:44 ` Ian Jackson 0 siblings, 1 reply; 11+ messages in thread From: George Dunlap @ 2013-11-21 15:08 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel, Ian Jackson, Mike On Thu, Nov 21, 2013 at 10:01 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Wed, 2013-11-20 at 18:50 +0000, George Dunlap wrote: >> On Wed, Nov 20, 2013 at 2:53 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Wed, 2013-11-20 at 14:37 +0000, Ian Jackson wrote: >> >> Ian Campbell writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax."): >> >> > On Mon, 2013-08-19 at 18:01 +0100, Ian Campbell wrote: >> >> > > On Mon, 2013-08-19 at 17:44 +0100, Ian Jackson wrote: >> >> > > > Do we know whether this new syntax is backwards-compatible ? >> >> > > >> >> > > Good question. Squeeze's ip(8) doesn't mention dev for "ip link show" in >> >> > > the synopsis by virtue of not defining DEVICE anywhere but it does show >> >> > > the use of dev later on and the tool accepts it. Likewise for "ip link >> >> > > set". Given this, as Mike says, makes it impossible to name a device >> >> > > "dev" it seems likely to be a documentation bug. >> >> > > >> >> > > "ip addr flush" is correctly documented as needing the dev (by saying >> >> > > "dev STRING" and not "DEVICE" , I didn't try without but I assume it >> >> > > works since our scripts do that right now. >> >> > > >> >> > > I wasn't easily able to come up with an older machine. >> >> > >> >> > Ian, was that a satisfactory explanation? >> >> >> >> Yes, it was. Sorry. >> >> >> >> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> >> > >> > Thanks. >> > >> > Now, WRT 4.4... >> > >> > With reference to bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 the >> > patch allows the creation of a vif named "dev". Which although I'm sure >> > useful it is easily worked around. But on the other hand we are early in >> > the freeze and this could easily be considered a bug fix. >> > >> > At this stage I'm leaning towards say lets take it. >> >> Should I say 'no' once just to let people know who's in charge? :-) >> >> I might be inclined to do so actually, just on principle; except that >> the best way to get this actually tested will be the upcoming test >> days, when (hopefully) it will be tested on all the major distros. If >> we put it off until 4.5, I doubt it will get much more testing than it >> would right now. >> >> One thing I'm not happy with the patch though -- it doesn't have a >> proper description of the problem and what the patch is actually doing >> in the changeset itself. Links to supplemental information are fine, >> I think, but we have to assume that 1) people will be browsing the >> source code offline, and 2) links will disappear; so the critical >> information needs to be included. > > Ack. > > 8>---------------------- > > From aed6743365d17c58561481ce1d74613f9a731746 Mon Sep 17 00:00:00 2001 > From: Mike <debian@good-with-numbers.com> > Date: Fri, 16 Aug 2013 15:31:43 +0100 > Subject: [PATCH] hotplug/Linux: update to new ip command syntax. > > The current usage prevents naming a vif "dev". Although the current syntax is > documented in Squeeze's ip(8) it appears that this was a documentation bug. > Newer versions of the man page describe the new syntax used here and Squeeze's > implementation accepts it as well. > > This is Debian bug #705659. > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=705659 > > Mike provided the initial patch to tools/hotplug/Linux/xen-network-common.sh > and Ian extended it to catch some cases in vif-* too. > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > Signed-off-by: Mike <debian@good-with-numbers.com> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Much better, thanks. Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com> > --- > v2: Updated commit message. > --- > tools/hotplug/Linux/vif-bridge | 2 +- > tools/hotplug/Linux/vif-nat | 2 +- > tools/hotplug/Linux/xen-network-common.sh | 14 +++++++------- > 3 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge > index 678262d..b7dcbd6 100644 > --- a/tools/hotplug/Linux/vif-bridge > +++ b/tools/hotplug/Linux/vif-bridge > @@ -72,7 +72,7 @@ else > fi > > RET=0 > -ip link show $bridge 1>/dev/null 2>&1 || RET=1 > +ip link show dev $bridge 1>/dev/null 2>&1 || RET=1 > if [ "$RET" -eq 1 ] > then > fatal "Could not find bridge device $bridge" > diff --git a/tools/hotplug/Linux/vif-nat b/tools/hotplug/Linux/vif-nat > index 8d29fb6..0b900d5 100644 > --- a/tools/hotplug/Linux/vif-nat > +++ b/tools/hotplug/Linux/vif-nat > @@ -170,7 +170,7 @@ case "$command" in > exit 0 > fi > > - do_or_die ip link set "${dev}" up arp on > + do_or_die ip link set dev "${dev}" up arp on > do_or_die ip addr add "$router_ip" dev "${dev}" > do_or_die ip route add "$vif_ip" dev "${dev}" src "$router_ip" > echo 1 >/proc/sys/net/ipv4/conf/${dev}/proxy_arp > diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh > index 50b8711..3c63c55 100644 > --- a/tools/hotplug/Linux/xen-network-common.sh > +++ b/tools/hotplug/Linux/xen-network-common.sh > @@ -85,18 +85,18 @@ _setup_bridge_port() { > local virtual="$2" > > # take interface down ... > - ip link set ${dev} down > + ip link set dev ${dev} down > > if [ $virtual -ne 0 ] ; then > # Initialise a dummy MAC address. We choose the numerically > # largest non-broadcast address to prevent the address getting > # stolen by an Ethernet bridge for STP purposes. > # (FE:FF:FF:FF:FF:FF) > - ip link set ${dev} address fe:ff:ff:ff:ff:ff || true > + ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true > fi > > # ... and configure it > - ip addr flush ${dev} > + ip address flush dev ${dev} > } > > setup_physical_bridge_port() { > @@ -125,20 +125,20 @@ add_to_bridge () { > > # Don't add $dev to $bridge if it's already on a bridge. > if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then > - ip link set ${dev} up || true > + ip link set dev ${dev} up || true > return > fi > brctl addif ${bridge} ${dev} > - ip link set ${dev} up > + ip link set dev ${dev} up > } > > # Usage: set_mtu bridge dev > set_mtu () { > local bridge=$1 > local dev=$2 > - mtu="`ip link show ${bridge}| awk '/mtu/ { print $5 }'`" > + mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`" > if [ -n "$mtu" ] && [ "$mtu" -gt 0 ] > then > - ip link set ${dev} mtu $mtu || : > + ip link set dev ${dev} mtu $mtu || : > fi > } > -- > 1.7.10.4 > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] hotplug/Linux: update to new ip command syntax. 2013-11-21 15:08 ` George Dunlap @ 2013-11-21 18:44 ` Ian Jackson 0 siblings, 0 replies; 11+ messages in thread From: Ian Jackson @ 2013-11-21 18:44 UTC (permalink / raw) To: George Dunlap; +Cc: xen-devel, Ian Campbell, Mike George Dunlap writes ("Re: [Xen-devel] [PATCH] hotplug/Linux: update to new ip command syntax."): > On Thu, Nov 21, 2013 at 10:01 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > From: Mike <debian@good-with-numbers.com> > > Date: Fri, 16 Aug 2013 15:31:43 +0100 > > Subject: [PATCH] hotplug/Linux: update to new ip command syntax. ... > > Signed-off-by: Ian Campbell <ijc@hellion.org.uk> > > Signed-off-by: Mike <debian@good-with-numbers.com> > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > Much better, thanks. > > Release-acked-by: George Dunlap <george.dunlap@eu.citrix.com> Applied, thanks. Ian. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-11-21 18:44 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-08-16 14:31 [PATCH] hotplug/Linux: update to new ip command syntax Ian Campbell 2013-08-16 19:30 ` Mike 2013-08-19 16:44 ` Ian Jackson 2013-08-19 17:01 ` Ian Campbell 2013-11-19 12:23 ` Ian Campbell 2013-11-20 14:37 ` Ian Jackson 2013-11-20 14:53 ` Ian Campbell 2013-11-20 18:50 ` George Dunlap 2013-11-21 10:01 ` Ian Campbell 2013-11-21 15:08 ` George Dunlap 2013-11-21 18:44 ` Ian Jackson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.