On Tue, 2007-09-25 at 12:33 +0200, Or Gerlitz wrote:
> Eli Cohen wrote:
> > Add checksum offload support to ipoib
>
> Can you clarify the relation between this patch to "[PATCHv3] IB/ipoib:
> HW checksum support" patch posted later by Michael? for example, I see
> that you patch makes IPoIB to publish the NETIF_F_IP_CSUM capability and
> Michael's one publishes NETIF_F_HW_CSUM, etc
These two patches are not related. Michael's patch relies on infinband's
icrc to not require checksum generation/validation while my patch relies
on HW's capability to insert checksum on outgoing packets and calculate
checksum of incoming packtes.
>
> > Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib.h
> > ===================================================================
> > --- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib.h
> > 2007-09-24 12:09:21.000000000 +0200
> > +++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib.h 2007-09-24
> > 12:49:00.000000000 +0200
> > @@ -86,6 +86,7 @@ enum {
> > IPOIB_MCAST_STARTED = 8,
> > IPOIB_FLAG_NETIF_STOPPED = 9,
> > IPOIB_FLAG_ADMIN_CM = 10,
> > + IPOIB_FLAG_RX_CSUM = 11,
> >
> > IPOIB_MAX_BACKOFF_SECONDS = 16,
> >
> > Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > ===================================================================
> > --- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > 2007-09-24 12:23:26.000000000 +0200
> > +++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_cm.c
> > 2007-09-24 13:05:21.000000000 +0200
> > @@ -1258,6 +1258,13 @@ static ssize_t set_mode(struct device *d
> > set_bit(IPOIB_FLAG_ADMIN_CM, &priv->flags);
> > ipoib_warn(priv, "enabling connected mode "
> > "will cause multicast packet drops\n");
> > +
> > + /* clear ipv6 flag too */
> > + dev->features &= ~NETIF_F_IP_CSUM;
> > +
> > + priv->tx_wr.send_flags &=
> > + ~(IB_SEND_UDP_TCP_CSUM | IB_SEND_IP_CSUM);
> > +
> > ipoib_flush_paths(dev);
> > return count;
> > }
> > @@ -1266,6 +1273,10 @@ static ssize_t set_mode(struct device *d
> > clear_bit(IPOIB_FLAG_ADMIN_CM, &priv->flags);
> > dev->mtu = min(priv->mcast_mtu, dev->mtu);
> > ipoib_flush_paths(dev);
> > +
> > + if (priv->ca->flags & IB_DEVICE_IP_CSUM)
> > + dev->features |= NETIF_F_IP_CSUM; /* ipv6 too */
>
> didn't you want to use NETIF_F_HW_CSUM here?
No, our hw does ip/udp/tcp checksum only.
>
> > Index: ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > ===================================================================
> > --- ofa_1_3_dev_kernel.orig/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > 2007-09-24 12:23:00.000000000 +0200
> > +++ ofa_1_3_dev_kernel/drivers/infiniband/ulp/ipoib/ipoib_main.c
> > 2007-09-24 13:04:52.000000000 +0200
> > @@ -1109,6 +1109,29 @@ int ipoib_add_pkey_attr(struct net_devic
> > return device_create_file(&dev->dev, &dev_attr_pkey);
> > }
> >
> > +static void set_tx_csum(struct net_device *dev)
> > +{
> > + struct ipoib_dev_priv *priv = netdev_priv(dev);
> > +
> > + if (test_bit(IPOIB_FLAG_ADMIN_CM, &priv->flags))
> > + return;
> > +
> > + if (!(priv->ca->flags & IB_DEVICE_IP_CSUM))
> > + return;
> > +
> > + dev->features |= NETIF_F_SG | NETIF_F_IP_CSUM; /* turn on ipv6 too */
> can you explain why this line belongs specifically to set_tx_csum() ?
>
> > +}
> > +
> > +static void set_rx_csum(struct net_device *dev)
> > +{
> > + struct ipoib_dev_priv *priv = netdev_priv(dev);
> > +
> > + if (!(priv->ca->flags & IB_DEVICE_IP_CSUM))
> > + return;
> > +
> > + set_bit(IPOIB_FLAG_RX_CSUM, &priv->flags);
> > +}
> > +
> > static struct net_device *ipoib_add_port(const char *format,
> > struct ib_device *hca, u8 port)
> > {
> > @@ -1165,6 +1188,9 @@ static struct net_device *ipoib_add_port
> > goto event_failed;
> > }
> >
> > + set_tx_csum(priv->dev);
> > + set_rx_csum(priv->dev);
>
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
