Re: Dropping NETIF_F_SG since no checksum feature.

["Michael S. Tsirkin" <mst@mellanox.co.il>]

Quoting Steven Whitehouse <steve@chygwyn.com>:
> > ssize_t tcp_sendpage(struct socket *sock, struct page *page, int offset,
> >                      size_t size, int flags)
> > {
> >         ssize_t res;
> >         struct sock *sk = sock->sk;
> > 
> >         if (!(sk->sk_route_caps & NETIF_F_SG) ||
> >             !(sk->sk_route_caps & NETIF_F_ALL_CSUM))
> >                 return sock_no_sendpage(sock, page, offset, size, flags);
> > 
> > 
> > So, it seems that if I set NETIF_F_SG but clear NETIF_F_ALL_CSUM,
> > data will be copied over rather than sent directly.
> > So why does dev.c have to force set NETIF_F_SG to off then?
> >
> I agree with that analysis,

So, would you Ack something like the following then?

======================

Enabling NETIF_F_SG without NETIF_F_ALL_CSUM actually seems to work fine by
doing an old-fashioned data copy in tcp_sendpage.
And for devices that do not calculate IP checksum in hardware (e.g. InfiniBand)
calculating the checksum for all packets in network driver is worse than have
the CPU piggyback the checksum compitation with the copy process.
Finally, note that NETIF_F_SG is necessary to be able to allocate skbs >
PAGE_SIZE on busy systems.

So, let's allow that combination, again, for drivers that want it.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

---

diff --git a/net/core/dev.c b/net/core/dev.c
index d4a1ec3..2d731a0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2930,14 +2930,6 @@ #endif
 		}
  	}
 
-	/* Fix illegal SG+CSUM combinations. */
-	if ((dev->features & NETIF_F_SG) &&
-	    !(dev->features & NETIF_F_ALL_CSUM)) {
-		printk(KERN_NOTICE "%s: Dropping NETIF_F_SG since no checksum feature.\n",
-		       dev->name);
-		dev->features &= ~NETIF_F_SG;
-	}
-
 	/* TSO requires that SG is present as well. */
 	if ((dev->features & NETIF_F_TSO) &&
 	    !(dev->features & NETIF_F_SG)) {

-- 
MST
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/