Re: Trying to adapt Olaf's vif MTU patch for EL5.6's 2.6.18-238.9 kernel - patch in-line.

[Digimer <linux@alteeve.com>]

On 04/16/2011 11:23 PM, Teck Choon Giam wrote:
> On Sun, Apr 17, 2011 at 9:44 AM, Digimer <linux@alteeve.com> wrote:
>> Hi all,
>>
>>  As per the request on ##xen, I'm reposting this with patches and
>> diff's inline. Please ignore my earlier post.
>>
>>  With a *lot* of help from Pasi yesterday, I was trying to get Olaf's
>> vif MTU patch
>> (http://lists.xensource.com/archives/html/xen-devel/2011-02/msg00352.html)
>> applied against the most recent EL5.6 (RHEL 5.6/CentOS 5.6) kernel
>> version 2.6.18-238.9.
>>
>>  The patch as-is didn't apply cleanly, as there seemed to be a line
>> shift issue with common.h and xenbus.c. I manually added the lines and
>> created a new patch, but I screwed something up and it fails to compile.
>> I should note that I am /not/ a C programmer. :)
>>
>>  Here is adapted patch:
>>
>> ====
>> --- linux-2.6.18.x86_64/drivers/xen/netback/common.h.orig       2011-04-15
>> 17:46:12.730677042 -0400
>> +++ linux-2.6.18.x86_64/drivers/xen/netback/common.h    2011-04-15
>> 17:57:57.490721792 -0400
>> @@ -76,8 +76,13 @@
>>        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;
>> +
>> +       /* Frontend feature information. */
>> +       u8 can_sg:1;
>> +       u8 gso:1;
>> +       u8 csum:1;
>>
>>        /* Internal feature information. */
>>        int can_queue:1;        /* can queue packets for receiver? */
>> @@ -110,6 +115,7 @@
>>
>>  void netif_disconnect(netif_t *netif);
>>
>> +void netif_set_features(netif_t *netif);
>>  netif_t *netif_alloc(domid_t domid, unsigned int handle);
>>  int netif_map(netif_t *netif, unsigned long tx_ring_ref,
>>              unsigned long rx_ring_ref, unsigned int evtchn);
>> @@ -142,7 +148,7 @@
>>  static inline int netbk_can_sg(struct net_device *dev)
>>  {
>>        netif_t *netif = netdev_priv(dev);
>> -       return netif->features & NETIF_F_SG;
>> +       return netif->can_sg;
>>  }
>>
>>  #endif /* __NETIF__BACKEND__COMMON_H__ */
>> --- linux-2.6.18.x86_64/drivers/xen/netback/interface.c.orig    2011-04-15
>> 17:46:57.205456542 -0400
>> +++ linux-2.6.18.x86_64/drivers/xen/netback/interface.c 2011-04-15
>> 17:50:46.027757042 -0400
>> @@ -90,34 +90,75 @@
>>        return 0;
>>  }
>>
>> -static int netbk_set_sg(struct net_device *dev, u32 data)
>> +void netif_set_features(netif_t *netif)
>>  {
>> +       struct net_device *dev = netif->dev;
>> +       int features = dev->features;
>> +
>> +       if (netif->can_sg)
>> +               features |= NETIF_F_SG;
>> +       if (netif->gso)
>> +               features |= NETIF_F_TSO;
>> +       if (netif->csum)
>> +               features |= NETIF_F_IP_CSUM;
>> +
>> +       features &= ~(netif->features_disabled);
>> +
>> +       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)
>> +{
>> +       netif_t *netif = netdev_priv(dev);
>>        if (data) {
>> -               netif_t *netif = netdev_priv(dev);
>> +               if (!netif->csum)
>> +                       return -ENOSYS;
>> +               netif->features_disabled &= ~NETIF_F_IP_CSUM;
>> +       } else {
>> +               netif->features_disabled |= NETIF_F_IP_CSUM;
>> +       }
>>
>> -               if (!(netif->features & NETIF_F_SG))
>> +       netif_set_features(netif);
>> +       return 0;
>> +}
>> +
>> +static int netbk_set_sg(struct net_device *dev, u32 data)
>> +{
>> +       netif_t *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;
>>        }
>>
>> -       return ethtool_op_set_sg(dev, data);
>> +       netif_set_features(netif);
>> +       return 0;
>>  }
>>
>>  static int netbk_set_tso(struct net_device *dev, u32 data)
>>  {
>> +       netif_t *netif = netdev_priv(dev);
>>        if (data) {
>> -               netif_t *netif = netdev_priv(dev);
>> -
>> -               if (!(netif->features & NETIF_F_TSO))
>> +               if (!netif->gso)
>>                        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 struct ethtool_ops network_ethtool_ops =
>>  {
>>        .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,
>> @@ -145,6 +186,8 @@
>>        memset(netif, 0, sizeof(*netif));
>>        netif->domid  = domid;
>>        netif->handle = handle;
>> +       netif->can_sg = 1;
>> +       netif->csum = 1;
>>        atomic_set(&netif->refcnt, 1);
>>        init_waitqueue_head(&netif->waiting_to_free);
>>        netif->dev = dev;
>> @@ -162,7 +205,8 @@
>>        dev->open            = net_open;
>>        dev->stop            = net_close;
>>        dev->change_mtu      = netbk_change_mtu;
>> -       dev->features        = NETIF_F_IP_CSUM;
>> +
>> +       netif_set_features(netif);
>>
>>        SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
>>
>> --- linux-2.6.18.x86_64/drivers/xen/netback/netback.c.orig      2011-04-15
>> 17:46:58.141515042 -0400
>> +++ linux-2.6.18.x86_64/drivers/xen/netback/netback.c   2011-04-15
>> 17:50:46.027757042 -0400
>> @@ -215,7 +215,7 @@
>>
>>  static inline int netbk_max_required_rx_slots(netif_t *netif)
>>  {
>> -       if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
>> +       if (netif->can_sg || netif->gso)
>>                return MAX_SKB_FRAGS + 2; /* header + extra_info + frags */
>>        return 1; /* all in one */
>>  }
>> --- linux-2.6.18.x86_64/drivers/xen/netback/xenbus.c.orig       2011-04-15
>> 17:46:59.217582292 -0400
>> +++ linux-2.6.18.x86_64/drivers/xen/netback/xenbus.c    2011-04-15
>> 18:13:20.700418792 -0400
>> @@ -345,6 +345,7 @@
>>
>>  static int connect_rings(struct backend_info *be)
>>  {
>> +       netif_t *netif = be->netif;
>>        struct xenbus_device *dev = be->dev;
>>        unsigned long tx_ring_ref, rx_ring_ref;
>>        unsigned int evtchn, rx_copy;
>> @@ -375,46 +376,40 @@
>>                                 dev->otherend);
>>                return err;
>>        }
>> -       be->netif->copying_receiver = !!rx_copy;
>> +       netif->copying_receiver = !!rx_copy;
>>
>> -       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
> 
> You need to end semi-colon for the above which I think your compile
> error output is showing I guess.
> 
>>                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)
>>                val = 0;
>> -       if (val) {
>> -               be->netif->features |= NETIF_F_SG;
>> -               be->netif->dev->features |= NETIF_F_SG;
>> -       }
>> +       netif->can_sg = !!val;
>>
>>        if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4", "%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-no-csum-offload",
>>                         "%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;
>> +
>> +       /* Set dev->features */
>> +       netif_set_features(netif);
>>
>>        netdev_features_change(be->netif->dev);
>>
>>        /* 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",
>> ====
>>
>>  And here is the compile output with errors:
>>
>> ====
>> drivers/xen/blkfront/blkfront.c: In function 'do_blkif_request':
>> drivers/xen/blkfront/blkfront.c:629: warning: format '%lx' expects type
>> 'long unsigned int', but argument 4 has type 'sector_t'
>> drivers/xen/blktap/blktapmain.c:145: warning: 'set_blkif_reqs' defined
>> but not used
>> drivers/xen/netback/xenbus.c: In function 'connect_rings':
>> drivers/xen/netback/xenbus.c:387: error: expected ';' before 'else'
> 
> Is the line 387 the one without the ; above?  I guess so :p
> 
> Thanks.
> 
> Kindest regards,
> Giam Teck Choon

Oh dear, now this is embarrassing!

I'll edit it and re-run the compile. I'll post a follow-up in an hour or
two. :P

Thanks!

-- 
Digimer
E-Mail: digimer@alteeve.com
AN!Whitepapers: http://alteeve.com
Node Assassin:  http://nodeassassin.org