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

["Durrant, Paul" <pdurrant@xxxxxxxxxxxx>]

> -----Original Message-----
> From: Ian Jackson <ian.jackson@xxxxxxxxxx>
> Sent: 05 August 2020 10:31
> To: paul@xxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Durrant, Paul <pdurrant@xxxxxxxxxxxx>; 
> 'Wei Liu' <wl@xxxxxxx>
> Subject: RE: [EXTERNAL] [PATCH v2 4/4] tools/hotplug: modify set_mtu() to 
> inform the frontend via
> xenstore
> 
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open
> attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> 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>
> ...
> > > 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.
> 
> Hmm.  How does this interact with driver domains ?  I think a driver
> domain might not have write access to this node.
> 

That's a good point; I think we will also need to actually write it from libxl 
first in that case.

> Is there a value we can store in it that won't break these Windows
> frontends, that libxl in the toolstack domain could write, before the
> hotplug script runs in the driver domain ?
> 
> > 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.
> 
> I agree that the default should come from the bridge.  Ideally there
> would be a way to override it in the config.
> 

Well, I guess we address the driver domain issue in this way too... I will add 
a patch to libxl to write the libxl_device_nic mtu value into xenstore, in both 
backend (where it should always have been) and frontend. I think the current 
setting of 1492 can be changed to 1500 safely (since nothing appears to 
currently use that value). The hotplug script should then have sufficient 
access to update, and a subsequent patch can add a mechanism to set the value 
from the config.

  Paul