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 11:13
> To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
> Cc: paul@xxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx; '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.
> 
> 
> 
> Durrant, Paul 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>
> ...
> > 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,
> 
> Do you mean libxl in dom0 or libxl in the driver domain ?
> 
> libxl contains code that runs in both contexts.
> 
> See device_hotplug in libxl_device.c, in particular notice
>     if (aodev->dev->backend_domid != domid) {
> 
> If you want the mtu to be read from the bridge, it can only be
> determined by the driver domain, because the bridge is in the driver
> domain.
> 
> In v2 of this series you arrange for the hotplug script to copy the
> mtu from the bridge into the frontend path.  That won't work because
> the hotplug script can't write to that xenstore node because (unlike a
> domo0 backend) a driver domain backend doesn't have write access to
> the frontend so can't create a new node there.
> 
> ISTM that it is correct that it is the hotplug script that does this
> interface setup.  If it weren't for this erroneous use of the frontend
> path I think the right design would be:
>   * toolstack libxl reads the config file to find whether there is an MTU
>   * toolstack libxl writes mtu node in backend iff one was in config
>     (and leaves the node absent otherwise)

This is where the 'subsequent patch' comes in (see the end of the email)...

>   * driver domain libxl runs hotplug script
>   * driver domain hotplug script looks for mtu in backend; if there
>     isn't one, it gets the value from the bridge and writes it to
>     the backend in xenstore
>   * driver domain backend driver reads mtu value from backend path
>   * guest domain frontend driver reads mtu value from backend path
>   * on domain save/migrate, toolstack libxl will record the mtu
>     value as the actual configuration so that the migrated domain
>     will get the same mtu
> 

That sounds right.

> I don't think I understand what (in these kind of terms) you are
> proposing, in order to support the frontends that want to read the mtu
> from the frontend.

We need some way for creating the frontend node such that the driver domain has 
write access. Presumably there is a suitable place in the toolstack instance of 
libxl to do this. This would mean we either need to write a sentinel 'invalid' 
value or write the default value. In the default case we could still leave the 
backend node absent so the hotplug script will still know whether or not a 
value was set in the cfg.

> 
> >  I think the current setting of 1492 can be changed to 1500 safely
> > (since nothing appears to currently use that value).
> 
> Right, that seems correct to me.
> 
> > 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.
> 
> I think what I am missing is how this "subsequent patch" would work ?
> Ie what design are we aiming for, that we are now implementing part
> of ?

The subsequent patch would be one that actually acquires the mtu value from the 
vif config, and adds documentation to xl-network-configuration.5.pod, since 
this is currently missing.

  Paul

> 
> Ian.