Re: [Xen-devel] [PATCH] netback: allow arbitrary mtu size until frontend connects

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Mon, 2011-02-07 at 09:03 +0000, Jan Beulich wrote:
> >>> On 06.02.11 at 14:42, Olaf Hering <olaf@xxxxxxxxx> wrote:
> > Allow arbitrary mtu size until the frontend is connected.  Once the
> > connection is established, adjust mtu by checking if the backend
> > supports the 'feature-sg'.  If the backend does support it, keep the
> > current mtu. Otherwise set it to the default value, which is 1500.
> > 
> > This helps the vif-bridge hotplug script to set the mtu size to 9000
> > while bringing up the guest.
> 
> Isn't this functionally the same as
> http://xenbits.xen.org/XCP/linux-2.6.32.pq.hg?file/043b76e4943c/xen-netback-Allow-setting-of-large-MTU-before-rings.patch
> or its pv-ops parent 
> https://git.kernel.org/?p=linux/kernel/git/jeremy/xen.git;a=commitdiff;h=bee2eec2355c4bf4e149a426d5e30527162de566
> (in which case it would seem preferable to pull in that change)?
> 
> Ian, looking at that patch of yours again, the adjustment to
> netbk_set_sg() seems a little odd: Why is it necessary to
> reduce dev->mtu unconditionally (i.e. independent of the
> value of "data") here?

Hmm. I don't recall. Seems like a bug looking at it now...

Luckily for me it appears to have been correct by Paul in
d532fa93d4eeabbfc0176a6a9a93b0d6ade3f6c4

Ian.

> 
> Jan
> 
> > Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> > 
> > ---
> >  drivers/xen/netback/common.h    |    8 ++++++++
> >  drivers/xen/netback/interface.c |    8 +++++++-
> >  drivers/xen/netback/xenbus.c    |    9 +++++++++
> >  3 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/common.h
> > +++ linux-2.6.18-xen.hg/drivers/xen/netback/common.h
> > @@ -82,6 +82,8 @@ typedef struct netif_st {
> >     u8 can_queue:1; /* can queue packets for receiver? */
> >     u8 copying_receiver:1;  /* copy packets to receiver?       */
> >  
> > +   u8 frontend_connected:1; /* is frontend already connected? */
> > +
> >     /* Allow netif_be_start_xmit() to peek ahead in the rx request ring. */
> >     RING_IDX rx_req_cons_peek;
> >  
> > @@ -217,4 +219,10 @@ static inline int netbk_can_sg(struct ne
> >     return netif->features & NETIF_F_SG;
> >  }
> >  
> > +static inline int netbk_frontend_connected(struct net_device *dev)
> > +{
> > +   netif_t *netif = netdev_priv(dev);
> > +   return !!netif->frontend_connected;
> > +}
> > +
> >  #endif /* __NETIF__BACKEND__COMMON_H__ */
> > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/interface.c
> > +++ linux-2.6.18-xen.hg/drivers/xen/netback/interface.c
> > @@ -83,9 +83,13 @@ static int net_close(struct net_device *
> >     return 0;
> >  }
> >  
> > +/* accept the max value if frontend is not yet connected */
> >  static int netbk_change_mtu(struct net_device *dev, int mtu)
> >  {
> > -   int max = netbk_can_sg(dev) ? 65535 - ETH_HLEN : ETH_DATA_LEN;
> > +   int max = 65535 - ETH_HLEN;
> > +
> > +   if (netbk_frontend_connected(dev))
> > +           max = netbk_can_sg(dev) ? max : ETH_DATA_LEN;
> >  
> >     if (mtu > max)
> >             return -EINVAL;
> > @@ -359,6 +363,8 @@ err_rx:
> >  
> >  void netif_disconnect(netif_t *netif)
> >  {
> > +   netif->frontend_connected = 0;
> > +
> >     if (netback_carrier_ok(netif)) {
> >             rtnl_lock();
> >             netback_carrier_off(netif);
> > --- linux-2.6.18-xen.hg.orig/drivers/xen/netback/xenbus.c
> > +++ linux-2.6.18-xen.hg/drivers/xen/netback/xenbus.c
> > @@ -329,6 +329,13 @@ static int xen_net_read_mac(struct xenbu
> >     return 0;
> >  }
> >  
> > +static void xen_net_adjust_mtu(netif_t *netif)
> > +{
> > +   if (!netif->dev->change_mtu)
> > +           return;
> > +   netif->dev->change_mtu(netif->dev, netif->dev->mtu);
> > +}
> > +
> >  static void connect(struct backend_info *be)
> >  {
> >     int err;
> > @@ -338,6 +345,8 @@ static void connect(struct backend_info
> >     if (err)
> >             return;
> >  
> > +   be->netif->frontend_connected = 1;
> > +   xen_net_adjust_mtu(be->netif);
> >     err = xen_net_read_mac(dev, be->netif->fe_dev_addr);
> >     if (err) {
> >             xenbus_dev_fatal(dev, err, "parsing %s/mac", dev->nodename);
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx 
> > http://lists.xensource.com/xen-devel 
> 
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel