RE: [PATCH v2 4/4] tools/hotplug: modify set_mtu() to inform the frontend via xenstore

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Ian Jackson <ian.jackson@xxxxxxxxxx>
> Sent: 04 August 2020 12:35
> To: paul@xxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Paul Durrant' <pdurrant@xxxxxxxxxx>; 
> 'Wei Liu' <wl@xxxxxxx>
> Subject: RE: [PATCH v2 4/4] tools/hotplug: modify set_mtu() to inform the 
> frontend via xenstore
> 
> Paul Durrant writes ("RE: [PATCH v2 4/4] tools/hotplug: modify set_mtu() to 
> inform the frontend via
> xenstore"):
> > > -----Original Message-----
> > > From: Ian Jackson <ian.jackson@xxxxxxxxxx>
> > > Sent: 04 August 2020 12:14
> > > To: Paul Durrant <paul@xxxxxxx>
> > > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Paul Durrant <pdurrant@xxxxxxxxxx>; 
> > > Wei Liu <wl@xxxxxxx>
> > > Subject: Re: [PATCH v2 4/4] tools/hotplug: modify set_mtu() to inform the 
> > > frontend via xenstore
> > >
> > > Paul Durrant writes ("[PATCH v2 4/4] tools/hotplug: modify set_mtu() to 
> > > inform the frontend via
> > > xenstore"):
> > > > +       XENBUS_PATH="/local/domain/$domid/device/vif/$devid"
> > > > +       xenstore_write "$XENBUS_PATH/mtu" ${mtu}
> > >
> > > It's surprising to me that this code doesn't have the xenbus path
> > > already in some variable.  But I guess from the fact that you've added
> > > this code, that it doesn't.
> >
> > It is set, but set to the backend path. For safety I guess it's probably 
> > best if I use a local in
> this instance. Can I keep your R-b
> > with such a change?
> 
> Oh, wow.  I hadn't realised that.  I take back my earlier R-b :-).
> 
> Can you please use a different variable name for the frontend path ?
> 

OK.

> ...
> 
> Actually.
> 
> This shouldn't be in the frontend at all, should it ?  In general the
> backend writes to the backend and the frontend to the frontend.
> 
> So maybe I need to take back my R-b of
>   [PATCH v2 3/4] public/io/netif: specify MTU override node
> 
> Sorry for the confusion.  I seem rather undercaffienated today.
> 

Too late. The xenstore node has been used by Windows frontends for the best 
part of a decade so we can't practically change the
path. Another way would be to also modify netback to simply echo the value from 
backend into frontend, but that seems rather
pointless.

Interestingly libxl does define an 'mtu' field for libxl_device_nic, which it 
sets to 1492 in libxl__device_nic_setdefault() but
never writes it into xenstore. There is even a comment:

/* nic->mtu = */

in libxl__nic_from_xenstore() which implies it should have been there, but 
isn't.
I still think picking up the MTU from the bridge is the better way though. 

  Paul

> Ian.