All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Durrant <xadimgnik@gmail.com>
To: "'Ian Jackson'" <ian.jackson@citrix.com>
Cc: xen-devel@lists.xenproject.org,
	'Paul Durrant' <pdurrant@amazon.com>, 'Wei Liu' <wl@xen.org>
Subject: RE: [PATCH v2 4/4] tools/hotplug: modify set_mtu() to inform the frontend via xenstore
Date: Tue, 4 Aug 2020 14:31:12 +0100	[thread overview]
Message-ID: <002801d66a63$85fb8c10$91f2a430$@xen.org> (raw)
In-Reply-To: <24361.18433.500622.984594@mariner.uk.xensource.com>

> -----Original Message-----
> From: Ian Jackson <ian.jackson@citrix.com>
> Sent: 04 August 2020 12:35
> To: paul@xen.org
> Cc: xen-devel@lists.xenproject.org; 'Paul Durrant' <pdurrant@amazon.com>; 'Wei Liu' <wl@xen.org>
> 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@citrix.com>
> > > Sent: 04 August 2020 12:14
> > > To: Paul Durrant <paul@xen.org>
> > > Cc: xen-devel@lists.xenproject.org; Paul Durrant <pdurrant@amazon.com>; Wei Liu <wl@xen.org>
> > > 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.



  reply	other threads:[~2020-08-04 13:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-03 12:49 [PATCH v2 0/4] tools: propagate bridge MTU to vif frontends Paul Durrant
2020-08-03 12:49 ` [PATCH v2 1/4] tools/hotplug: add remove_from_bridge() and improve debug output Paul Durrant
2020-08-04 11:06   ` Ian Jackson
2020-08-03 12:49 ` [PATCH v2 2/4] tools/hotplug: combine add/online and remove/offline in vif-bridge Paul Durrant
2020-08-04 11:08   ` Ian Jackson
2020-08-03 12:49 ` [PATCH v2 3/4] public/io/netif: specify MTU override node Paul Durrant
2020-08-04 11:10   ` Ian Jackson
2020-08-03 12:49 ` [PATCH v2 4/4] tools/hotplug: modify set_mtu() to inform the frontend via xenstore Paul Durrant
2020-08-04 11:13   ` Ian Jackson
2020-08-04 11:20     ` Paul Durrant
2020-08-04 11:35       ` Ian Jackson
2020-08-04 13:31         ` Paul Durrant [this message]
2020-08-05  9:30           ` Ian Jackson
2020-08-05  9:44             ` Durrant, Paul
2020-08-05 10:13               ` Ian Jackson
2020-08-05 10:42                 ` Durrant, Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='002801d66a63$85fb8c10$91f2a430$@xen.org' \
    --to=xadimgnik@gmail.com \
    --cc=ian.jackson@citrix.com \
    --cc=paul@xen.org \
    --cc=pdurrant@amazon.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.