[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] RE: [PATCH] Make frontend features distinct from netback feature flags.



I notice there is a 1 char typo in this patch. See below for details. I'll send 
a corrected patch shortly.

  Paul

> -----Original Message-----
> From: Paul Durrant
> Sent: 02 July 2010 10:28
> To: xen-devel@xxxxxxxxxxxxxxxxxxx; jeremy@xxxxxxxx
> Cc: Paul Durrant; Ian Campbell
> Subject: [PATCH] Make frontend features distinct from netback
> feature flags.
> 
> Make sure that if a feature flag is disabled by ethtool on netback
> that we do not gratuitously re-enabled it when we check the frontend
> features during ring connection.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  drivers/xen/netback/common.h    |   15 ++++++---
>  drivers/xen/netback/interface.c |   68
> ++++++++++++++++++++++++++++++--------
>  drivers/xen/netback/netback.c   |    2 +-
>  drivers/xen/netback/xenbus.c    |   51 +++++++++++-----------------
> -
>  4 files changed, 83 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/xen/netback/common.h
> b/drivers/xen/netback/common.h
> index 1cbc4ff..a673331 100644
> --- a/drivers/xen/netback/common.h
> +++ b/drivers/xen/netback/common.h
> @@ -76,14 +76,18 @@ struct xen_netif {
>       struct vm_struct *tx_comms_area;
>       struct vm_struct *rx_comms_area;
> 
> -     /* Set of features that can be turned on in dev->features. */
> -     int features;
> +    /* Flags that must not be set in dev->features */
> +    int features_disabled;
> 
> -     int smart_poll;
> +     /* Frontend feature information. */
> +    u8 can_sg:1;
> +    u8 gso:1;
> +    u8 gso_prefix:1;
> +    u8 csum:1;
> +    u8 smart_poll:1;
> 
>       /* Internal feature information. */
>       u8 can_queue:1;     /* can queue packets for receiver?
> */
> -     u8 gso_prefix:1;    /* use a prefix segment for GSO
> information */
> 
>       /* Allow netif_be_start_xmit() to peek ahead in the rx
> request
>        * ring.  This is a prediction of what rx_req_cons will be
> once
> @@ -189,6 +193,7 @@ void netif_accel_init(void);
> 
>  void netif_disconnect(struct xen_netif *netif);
> 
> +void netif_set_features(struct xen_netif *netif);
>  struct xen_netif *netif_alloc(struct device *parent, domid_t domid,
> unsigned int handle);
>  int netif_map(struct xen_netif *netif, unsigned long tx_ring_ref,
>             unsigned long rx_ring_ref, unsigned int evtchn);
> @@ -225,7 +230,7 @@ static inline int netbk_can_queue(struct
> net_device *dev)
>  static inline int netbk_can_sg(struct net_device *dev)
>  {
>       struct xen_netif *netif = netdev_priv(dev);
> -     return netif->features & NETIF_F_SG;
> +     return netif->can_sg;
>  }
> 
>  struct pending_tx_info {
> diff --git a/drivers/xen/netback/interface.c
> b/drivers/xen/netback/interface.c
> index 172ef4c..2e8508a 100644
> --- a/drivers/xen/netback/interface.c
> +++ b/drivers/xen/netback/interface.c
> @@ -121,31 +121,69 @@ static int netbk_change_mtu(struct net_device
> *dev, int mtu)
>       return 0;
>  }
> 
> -static int netbk_set_sg(struct net_device *dev, u32 data)
> +void netif_set_features(struct xen_netif *netif)
>  {
> -     if (data) {
> -             struct xen_netif *netif = netdev_priv(dev);
> +     struct net_device *dev = netif->dev;
> +     int features = dev->features;
> +
> +     if (netif->can_sg)
> +             features |= NETIF_F_SG;
> +     if (netif->gso || netif->gso_prefix)
> +             features |= NETIF_F_TSO;
> +     if (netif->csum)
> +             features |= NETIF_F_IP_CSUM;
> +
> +     features &= ~(netif->features_disabled);
> 
> -             if (!(netif->features & NETIF_F_SG))
> +     if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
> +             dev->mtu = ETH_DATA_LEN;
> +
> +     dev->features = features;
> +}
> +
> +static int netbk_set_tx_csum(struct net_device *dev, u32 data)
> +{
> +     struct xen_netif *netif = netdev_priv(dev);
> +     if (data) {
> +             if (!netif->csum)
>                       return -ENOSYS;
> +             netif->features_disabled &= ~NETIF_F_IP_CSUM;
> +     } else {
> +             netif->features_disabled |= NETIF_F_IP_CSUM;
>       }
> 
> -     if (dev->mtu > ETH_DATA_LEN)
> -             dev->mtu = ETH_DATA_LEN;
> +     netif_set_features(netif);
> +     return 0;
> +}
> 
> -     return ethtool_op_set_sg(dev, data);
> +static int netbk_set_sg(struct net_device *dev, u32 data)
> +{
> +     struct xen_netif *netif = netdev_priv(dev);
> +     if (data) {
> +             if (!netif->can_sg)
> +                     return -ENOSYS;
> +             netif->features_disabled &= ~NETIF_F_SG;
> +     } else {
> +             netif->features_disabled |= NETIF_F_SG;
> +     }
> +
> +     netif_set_features(netif);
> +     return 0;
>  }
> 
>  static int netbk_set_tso(struct net_device *dev, u32 data)
>  {
> +     struct xen_netif *netif = netdev_priv(dev);
>       if (data) {
> -             struct xen_netif *netif = netdev_priv(dev);
> -
> -             if (!(netif->features & NETIF_F_TSO))
> +             if (!netif->gso && !netif->gso_prefix)
>                       return -ENOSYS;
> +             netif->features_disabled &= ~NETIF_F_TSO;
> +     } else {
> +             netif->features_disabled |= NETIF_F_TSO;
>       }
> 
> -     return ethtool_op_set_tso(dev, data);
> +     netif_set_features(netif);
> +     return 0;
>  }
> 
>  static void netbk_get_drvinfo(struct net_device *dev,
> @@ -200,7 +238,7 @@ static struct ethtool_ops network_ethtool_ops =
>       .get_drvinfo = netbk_get_drvinfo,
> 
>       .get_tx_csum = ethtool_op_get_tx_csum,
> -     .set_tx_csum = ethtool_op_set_tx_csum,
> +     .set_tx_csum = netbk_set_tx_csum,
>       .get_sg = ethtool_op_get_sg,
>       .set_sg = netbk_set_sg,
>       .get_tso = ethtool_op_get_tso,
> @@ -242,7 +280,8 @@ struct xen_netif *netif_alloc(struct device
> *parent, domid_t domid, unsigned int
>       netif->domid  = domid;
>       netif->group  = -1;
>       netif->handle = handle;
> -     netif->features = NETIF_F_SG;
> +     netif->can_sg = 1;
> +     netif->csum = 1;
>       atomic_set(&netif->refcnt, 1);
>       init_waitqueue_head(&netif->waiting_to_free);
>       netif->dev = dev;
> @@ -259,8 +298,7 @@ struct xen_netif *netif_alloc(struct device
> *parent, domid_t domid, unsigned int
>       init_timer(&netif->tx_queue_timeout);
> 
>       dev->netdev_ops = &netback_ops;
> -     dev->features   = NETIF_F_IP_CSUM|NETIF_F_SG;
> -
> +     netif_set_features(netif);
>       SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
> 
>       dev->tx_queue_len = netbk_queue_length;
> diff --git a/drivers/xen/netback/netback.c
> b/drivers/xen/netback/netback.c
> index 93f0686..426aada 100644
> --- a/drivers/xen/netback/netback.c
> +++ b/drivers/xen/netback/netback.c
> @@ -238,7 +238,7 @@ static struct sk_buff *netbk_copy_skb(struct
> sk_buff *skb)
> 
>  static inline int netbk_max_required_rx_slots(struct xen_netif
> *netif)
>  {
> -     if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
> +     if (netif->can_sg || netif->gso || netif->gso_prefix)
>               return MAX_SKB_FRAGS + 2; /* header + extra_info +
> frags */
>       return 1; /* all in one */
>  }
> diff --git a/drivers/xen/netback/xenbus.c
> b/drivers/xen/netback/xenbus.c
> index 74c035b..81b3a96 100644
> --- a/drivers/xen/netback/xenbus.c
> +++ b/drivers/xen/netback/xenbus.c
> @@ -412,6 +412,7 @@ static void connect(struct backend_info *be)
> 
>  static int connect_rings(struct backend_info *be)
>  {
> +     struct xen_netif *netif = be->netif;
>       struct xenbus_device *dev = be->dev;
>       unsigned long tx_ring_ref, rx_ring_ref;
>       unsigned int evtchn, rx_copy;
> @@ -445,61 +446,47 @@ static int connect_rings(struct backend_info
> *be)
>       if (!rx_copy)
>               return -EOPNOTSUPP;
> 
> -     if (be->netif->dev->tx_queue_len != 0) {
> +     if (netif->dev->tx_queue_len != 0) {
>               if (xenbus_scanf(XBT_NIL, dev->otherend,
>                                "feature-rx-notify", "%d", &val) < 0)
>                       val = 0;
>               if (val)
> -                     be->netif->can_queue = 1;
> +                     netif->can_queue = 1;
>               else
>                       /* Must be non-zero for pfifo_fast to work. */
> -                     be->netif->dev->tx_queue_len = 1;
> +                     netif->dev->tx_queue_len = 1;
>       }
> 
> -     if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d",
> &val) < 0)
> +     if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> +                      "%d", &val) < 0)
>               val = 0;
> -     if (!val) {
> -             be->netif->features &= ~NETIF_F_SG;
> -             be->netif->dev->features &= ~NETIF_F_SG;
> -             if (be->netif->dev->mtu > ETH_DATA_LEN)
> -                     be->netif->dev->mtu = ETH_DATA_LEN;
> -     }
> +     netif->can_sg = !!val;
> 
>       if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> -                     "%d", &val) < 0)
> +                      "%d", &val) < 0)
>               val = 0;
> -     if (val) {
> -             be->netif->features |= NETIF_F_TSO;
> -             be->netif->dev->features |= NETIF_F_TSO;
> -     }
> +     netif->gso = !!val;
> 
>       if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-
> prefix",
> -                     "%d", &val) < 0)
> +                      "%d", &val) < 0)
>               val = 0;
> -     if (val) {
> -             be->netif->features |= NETIF_F_TSO;
> -             be->netif->dev->features |= NETIF_F_TSO;
> -             be->netif->gso_prefix = 1;
> -     }
> +     netif->gso_prefix = !!val;
> 
>       if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-
> offload",
> -                     "%d", &val) < 0)
> +                      "%d", &val) < 0)
>               val = 0;
> -     if (val) {
> -             be->netif->features &= ~NETIF_F_IP_CSUM;
> -             be->netif->dev->features &= ~NETIF_F_IP_CSUM;
> -     }
> +     netif->csum = !!val;

This should be !val.

> 
>       if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-
> poll",
> -                     "%d", &val) < 0)
> +                      "%d", &val) < 0)
>               val = 0;
> -     if (val)
> -             be->netif->smart_poll = 1;
> -     else
> -             be->netif->smart_poll = 0;
> +     netif->smart_poll = !!val;
> +
> +     /* Set dev->features */
> +     netif_set_features(netif);
> 
>       /* Map the shared frame, irq etc. */
> -     err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
> +     err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
>       if (err) {
>               xenbus_dev_fatal(dev, err,
>                                "mapping shared-frames %lu/%lu port
> %u",
> --
> 1.5.6.5


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.