Hi Anthony,
On Tue, Apr 12, 2022 at 02:03:17PM +0100, Anthony PERARD wrote:
> Hi James,
>
> On Tue, Mar 01, 2022 at 09:35:13AM +0000, James Dingwall wrote:
> > The set_mtu() function of xen-network-common.sh currently has this code:
> >
> > if [ ${type_if} = vif ]
> > then
> > local dev_=${dev#vif}
> > local domid=${dev_%.*}
> > local devid=${dev_#*.}
> >
> > local FRONTEND_PATH="/local/domain/$domid/device/vif/$devid"
> >
> > xenstore_write "$FRONTEND_PATH/mtu" ${mtu}
> > fi
> >
> > This works fine if the device has its default name but if the xen config
> > defines the vifname parameter the FRONTEND_PATH is incorrectly constructed.
> > Learn the frontend path by reading the appropriate value from the backend.
>
> The patch looks fine, thanks. It is only missing a line
> "Signed-off-by: your_name <your_email>" at the end of the description.
> The meaning of this line is described in the file CONTRIBUTING, section
> "Developer's Certificate of Origin".
>
Thank you for your feedback. I've updated the patch as suggested. I've also
incorporated two other changes, one is a simple style change for consistency,
the other is to change a the test for a valid mtu from > 0 to >= 68. I can
resubmit the original patch if either of these are a problem.
Thanks,
James
commit 03ad5670f8a7402e30b288a55d088e87685cd1a1
Author: James Dingwall <[email protected]>
Date: Tue Apr 19 12:45:31 2022 +0100
The set_mtu() function of xen-network-common.sh currently has this code:
if [ ${type_if} = vif ]
then
local dev_=${dev#vif}
local domid=${dev_%.*}
local devid=${dev_#*.}
local FRONTEND_PATH="/local/domain/$domid/device/vif/$devid"
xenstore_write "$FRONTEND_PATH/mtu" ${mtu}
fi
This works fine if the device has its default name but if the xen config
defines the vifname parameter the FRONTEND_PATH is incorrectly constructed.
Learn the frontend path by reading the appropriate value from the backend.
Also change use of `...` to $(...) for a consistent style in the script
and adjust the valid check from `mtu > 0` to `mtu >= 68` per RFC 791.
Signed-off-by: James Dingwall <[email protected]>
diff --git a/tools/hotplug/Linux/xen-network-common.sh b/tools/hotplug/Linux/xen-network-common.sh
index 42fa704e8d..9a382c39f4 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -171,24 +171,20 @@ set_mtu () {
local mtu=$(xenstore_read_default "$XENBUS_PATH/mtu" "")
if [ -z "$mtu" ]
then
- mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`"
+ mtu="$(ip link show dev ${bridge}| awk '/mtu/ { print $5 }')"
if [ -n "$mtu" ]
then
log debug "$bridge MTU is $mtu"
fi
fi
- if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]
+ if [ -n "$mtu" ] && [ "$mtu" -ge 68 ]
then
log debug "setting $dev MTU to $mtu"
ip link set dev ${dev} mtu ${mtu} || :
if [ ${type_if} = vif ]
then
- local dev_=${dev#vif}
- local domid=${dev_%.*}
- local devid=${dev_#*.}
-
- local FRONTEND_PATH="/local/domain/$domid/device/vif/$devid"
+ local FRONTEND_PATH="$(xenstore_read "$XENBUS_PATH/frontend")"
xenstore_write "$FRONTEND_PATH/mtu" ${mtu}
fi
