Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] af_packet: replace struct pgv with char **
From: Changli Gao @ 2010-11-30 14:30 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Jiri Pirko, Neil Horman, netdev
In-Reply-To: <1291126568.2904.85.camel@edumazet-laptop>

On Tue, Nov 30, 2010 at 10:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 30 novembre 2010 à 21:57 +0800, Changli Gao a écrit :
>> As we can check if an address is vmalloc address with is_vmalloc_addr(),
>> we can replace struct pgv with char **. Then we may get more pg_vecs.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>> ---
>>  net/packet/af_packet.c |   50 ++++++++++++++++---------------------------------
>>  1 file changed, 17 insertions(+), 33 deletions(-)
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index 0171b20..a26f981 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -164,14 +164,8 @@ struct packet_mreq_max {
>>  static int packet_set_ring(struct sock *sk, struct tpacket_req *req,
>>               int closing, int tx_ring);
>>
>> -#define PGV_FROM_VMALLOC 1
>> -struct pgv {
>> -     char *buffer;
>> -     unsigned char flags;
>> -};
>> -
>
> This patch is premature.
>
> Also, keep the struct pgv, even if it has a single field, since having
> types is good to read the sources.
>
> We could have 'void *' or 'char *' everywhere, it is not nice...
>
>
> -       pg_vec = kcalloc(block_nr, sizeof(struct pgv), GFP_KERNEL);
> +       pg_vec = kcalloc(block_nr, sizeof(char *), GFP_KERNEL);
>
> I prefer to have :
>
>       pg_vec = kcalloc(block_nr, sizeof(struct pgv), GFP_KERNEL);
>

OK. I'll respin it and the previous one in the same serial to address
the issues mentioned by you. Thanks.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH 1/2] af_packet: use vmalloc_to_page() instead for the addresss returned by vmalloc()
From: Neil Horman @ 2010-11-30 14:37 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Changli Gao, David S. Miller, Jiri Pirko, netdev
In-Reply-To: <1291126341.2904.82.camel@edumazet-laptop>

On Tue, Nov 30, 2010 at 03:12:21PM +0100, Eric Dumazet wrote:
> Le mardi 30 novembre 2010 à 21:56 +0800, Changli Gao a écrit :
> > The following commit causes the pgv->buffer may point to the memory
> > returned by vmalloc(). And we can't use virt_to_page() for the vmalloc
> > address.
> > 
> > This patch introduces a new inline function pgv_to_page(), which calls
> > vmalloc_to_page() for the vmalloc address, and virt_to_page() for the
> > __get_free_pages address.
> > 
> >     commit 0e3125c755445664f00ad036e4fc2cd32fd52877
> >     Author: Neil Horman <nhorman@tuxdriver.com>
> >     Date:   Tue Nov 16 10:26:47 2010 -0800
> > 
> >     packet: Enhance AF_PACKET implementation to not require high order contiguous memory allocation (v4)
> > 
> 
> nice catch.
> 
Ouch, yes, thanks.

> > Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> > ---
> >  net/packet/af_packet.c |   21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index 422705d..0171b20 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -224,6 +224,13 @@ struct packet_skb_cb {
> >  
> >  #define PACKET_SKB_CB(__skb)	((struct packet_skb_cb *)((__skb)->cb))
> >  
> > +static inline struct page *pgv_to_page(void *addr)
> > +{
> > +	if (is_vmalloc_addr(addr))
> > +		return vmalloc_to_page(addr);
> 
> Hmm, I am wondering if calling vmalloc_to_page(addr) several times for
> each packet is not too expensive ? I believe it is.
> 
> What about caching "struct page *" pointer somewhere ?
> 
> Then later we have :
> 
> > -		p_start = virt_to_page(h.raw);
> > -		p_end = virt_to_page(h_end);
> > +		p_start = pgv_to_page(h.raw);
> > +		p_end = pgv_to_page(h_end);
> >  		while (p_start <= p_end) {
> >  			flush_dcache_page(p_start);
> >  			p_start++;
> 
> This was OK before Neil patch... after vmalloc(), assumption that
> p_start can be incremented is completely wrong.
> 
> To fix this, we need something else than your patch.
> 
Off the top of my head, I would think that pgv_to_page could be prototyped such
that it could accept addr, offset and struct page ** arguments.  That way we can
track the current page that we're mapped to, lowering the number of calls to
vmalloc_to_page, and we can still use an increment like we do above (as long as
its wrapped in a subsequent call to pgv_to_page)
Neil

> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* [PATCH] net: Fix drivers advertising HW_CSUM feature to use csum_start
From: MichałMirosław @ 2010-11-30 14:23 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, Jesse Gross
In-Reply-To: <fddashfjkasdhklfjashlkjfs@rechot.qmqm.pl>

Some drivers are using skb_transport_offset(skb) instead of skb->csum_start.
This does not matter now, but if someone implements checksumming of
encapsulated packets then this will break silently.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/atl1c/atl1c_main.c    |    2 +-
 drivers/net/atl1e/atl1e_main.c    |    2 +-
 drivers/net/atlx/atl1.c           |    2 +-
 drivers/net/cassini.c             |    2 +-
 drivers/net/e1000/e1000_main.c    |    2 +-
 drivers/net/e1000e/netdev.c       |    2 +-
 drivers/net/enic/enic_main.c      |    2 +-
 drivers/net/ixgb/ixgb_main.c      |    2 +-
 drivers/net/ll_temac_main.c       |    2 +-
 drivers/net/macvtap.c             |    3 +--
 drivers/net/myri10ge/myri10ge.c   |    2 +-
 drivers/net/niu.c                 |    2 +-
 drivers/net/skge.c                |    2 +-
 drivers/net/sungem.c              |    2 +-
 drivers/net/sunhme.c              |    2 +-
 drivers/net/tun.c                 |    2 +-
 drivers/net/usb/smsc95xx.c        |    7 +++----
 drivers/net/virtio_net.c          |    2 +-
 drivers/net/vmxnet3/vmxnet3_drv.c |    4 ++--
 include/linux/skbuff.h            |    5 +++++
 20 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/net/atl1c/atl1c_main.c b/drivers/net/atl1c/atl1c_main.c
index 09b099b..e48ea95 100644
--- a/drivers/net/atl1c/atl1c_main.c
+++ b/drivers/net/atl1c/atl1c_main.c
@@ -2078,7 +2078,7 @@ static int atl1c_tso_csum(struct atl1c_adapter *adapter,
 check_sum:
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		u8 css, cso;
-		cso = skb_transport_offset(skb);
+		cso = skb_checksum_start_offset(skb);
 
 		if (unlikely(cso & 0x1)) {
 			if (netif_msg_tx_err(adapter))
diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c
index ef6349b..e28f8ba 100644
--- a/drivers/net/atl1e/atl1e_main.c
+++ b/drivers/net/atl1e/atl1e_main.c
@@ -1649,7 +1649,7 @@ check_sum:
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		u8 css, cso;
 
-		cso = skb_transport_offset(skb);
+		cso = skb_checksum_start_offset(skb);
 		if (unlikely(cso & 0x1)) {
 			netdev_err(adapter->netdev,
 				   "payload offset should not ant event number\n");
diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 5336310..def8df8 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2174,7 +2174,7 @@ static int atl1_tx_csum(struct atl1_adapter *adapter, struct sk_buff *skb,
 	u8 css, cso;
 
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
-		css = (u8) (skb->csum_start - skb_headroom(skb));
+		css = skb_checksum_start_offset(skb);
 		cso = css + (u8) skb->csum_offset;
 		if (unlikely(css & 0x1)) {
 			/* L1 hardware requires an even number here */
diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c
index d6b6d6a..35568c1 100644
--- a/drivers/net/cassini.c
+++ b/drivers/net/cassini.c
@@ -2788,7 +2788,7 @@ static inline int cas_xmit_tx_ringN(struct cas *cp, int ring,
 
 	ctrl = 0;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const u64 csum_start_off = skb_transport_offset(skb);
+		const u64 csum_start_off = skb_checksum_start_offset(skb);
 		const u64 csum_stuff_off = csum_start_off + skb->csum_offset;
 
 		ctrl =  TX_DESC_CSUM_EN |
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index dcb7f82..9426e04 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -2720,7 +2720,7 @@ static bool e1000_tx_csum(struct e1000_adapter *adapter,
 		break;
 	}
 
-	css = skb_transport_offset(skb);
+	css = skb_checksum_start_offset(skb);
 
 	i = tx_ring->next_to_use;
 	buffer_info = &tx_ring->buffer_info[i];
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 0adcb79..9bb9406 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4473,7 +4473,7 @@ static bool e1000_tx_csum(struct e1000_adapter *adapter, struct sk_buff *skb)
 		break;
 	}
 
-	css = skb_transport_offset(skb);
+	css = skb_checksum_start_offset(skb);
 
 	i = tx_ring->next_to_use;
 	buffer_info = &tx_ring->buffer_info[i];
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 9f293fa..78e9d8b 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -702,7 +702,7 @@ static inline void enic_queue_wq_skb_csum_l4(struct enic *enic,
 {
 	unsigned int head_len = skb_headlen(skb);
 	unsigned int len_left = skb->len - head_len;
-	unsigned int hdr_len = skb_transport_offset(skb);
+	unsigned int hdr_len = skb_checksum_start_offset(skb);
 	unsigned int csum_offset = hdr_len + skb->csum_offset;
 	int eop = (len_left == 0);
 
diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
index 211a169..23ec89e 100644
--- a/drivers/net/ixgb/ixgb_main.c
+++ b/drivers/net/ixgb/ixgb_main.c
@@ -1250,7 +1250,7 @@ ixgb_tx_csum(struct ixgb_adapter *adapter, struct sk_buff *skb)
 
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		struct ixgb_buffer *buffer_info;
-		css = skb_transport_offset(skb);
+		css = skb_checksum_start_offset(skb);
 		cso = css + skb->csum_offset;
 
 		i = adapter->tx_ring.next_to_use;
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index 9f8e702..e2c2a72 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -692,7 +692,7 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	cur_p->app0 = 0;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		unsigned int csum_start_off = skb_transport_offset(skb);
+		unsigned int csum_start_off = skb_checksum_start_offset(skb);
 		unsigned int csum_index_off = csum_start_off + skb->csum_offset;
 
 		cur_p->app0 |= 1; /* TX Checksum Enabled */
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 4256727..21845af 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -504,8 +504,7 @@ static int macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		vnet_hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-		vnet_hdr->csum_start = skb->csum_start -
-					skb_headroom(skb);
+		vnet_hdr->csum_start = skb_checksum_start_offset(skb);
 		vnet_hdr->csum_offset = skb->csum_offset;
 	} /* else everything is zero */
 
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 8524cc4..c504e29 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -2736,7 +2736,7 @@ again:
 	odd_flag = 0;
 	flags = (MXGEFW_FLAGS_NO_TSO | MXGEFW_FLAGS_FIRST);
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
-		cksum_offset = skb_transport_offset(skb);
+		cksum_offset = skb_checksum_start_offset(skb);
 		pseudo_hdr_offset = cksum_offset + skb->csum_offset;
 		/* If the headers are excessively large, then we must
 		 * fall back to a software checksum */
diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 781e368..f54d358 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -6589,7 +6589,7 @@ static u64 niu_compute_tx_flags(struct sk_buff *skb, struct ethhdr *ehdr,
 			     (ip_proto == IPPROTO_UDP ?
 			      TXHDR_CSUM_UDP : TXHDR_CSUM_SCTP));
 
-		start = skb_transport_offset(skb) -
+		start = skb_checksum_start_offset(skb) -
 			(pad_bytes + sizeof(struct tx_pkt_hdr));
 		stuff = start + skb->csum_offset;
 
diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 220e039..1959438 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -2764,7 +2764,7 @@ static netdev_tx_t skge_xmit_frame(struct sk_buff *skb,
 	td->dma_hi = map >> 32;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const int offset = skb_transport_offset(skb);
+		const int offset = skb_checksum_start_offset(skb);
 
 		/* This seems backwards, but it is what the sk98lin
 		 * does.  Looks like hardware is wrong?
diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
index 4ceb3cf..4842fca 100644
--- a/drivers/net/sungem.c
+++ b/drivers/net/sungem.c
@@ -1004,7 +1004,7 @@ static netdev_tx_t gem_start_xmit(struct sk_buff *skb,
 
 	ctrl = 0;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const u64 csum_start_off = skb_transport_offset(skb);
+		const u64 csum_start_off = skb_checksum_start_offset(skb);
 		const u64 csum_stuff_off = csum_start_off + skb->csum_offset;
 
 		ctrl = (TXDCTRL_CENAB |
diff --git a/drivers/net/sunhme.c b/drivers/net/sunhme.c
index 5e28c41..55bbb9c 100644
--- a/drivers/net/sunhme.c
+++ b/drivers/net/sunhme.c
@@ -2266,7 +2266,7 @@ static netdev_tx_t happy_meal_start_xmit(struct sk_buff *skb,
 
 	tx_flags = TXFLAG_OWN;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const u32 csum_start_off = skb_transport_offset(skb);
+		const u32 csum_start_off = skb_checksum_start_offset(skb);
 		const u32 csum_stuff_off = csum_start_off + skb->csum_offset;
 
 		tx_flags = (TXFLAG_OWN | TXFLAG_CSENABLE |
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 55f3a3e..7599c45 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -757,7 +757,7 @@ static __inline__ ssize_t tun_put_user(struct tun_struct *tun,
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
 			gso.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-			gso.csum_start = skb->csum_start - skb_headroom(skb);
+			gso.csum_start = skb_checksum_start_offset(skb);
 			gso.csum_offset = skb->csum_offset;
 		} /* else everything is zero */
 
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 65cb1ab..bc86f4b 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1163,9 +1163,8 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 static u32 smsc95xx_calc_csum_preamble(struct sk_buff *skb)
 {
-	int len = skb->data - skb->head;
-	u16 high_16 = (u16)(skb->csum_offset + skb->csum_start - len);
-	u16 low_16 = (u16)(skb->csum_start - len);
+	u16 low_16 = (u16)skb_checksum_start_offset(skb);
+	u16 high_16 = low_16 + skb->csum_offset;
 	return (high_16 << 16) | low_16;
 }
 
@@ -1193,7 +1192,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 		if (skb->len <= 45) {
 			/* workaround - hardware tx checksum does not work
 			 * properly with extremely small packets */
-			long csstart = skb->csum_start - skb_headroom(skb);
+			long csstart = skb_checksum_start_offset(skb);
 			__wsum calc = csum_partial(skb->data + csstart,
 				skb->len - csstart, 0);
 			*((__sum16 *)(skb->data + csstart
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b6d4028..90a23e4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -519,7 +519,7 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		hdr->hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-		hdr->hdr.csum_start = skb->csum_start - skb_headroom(skb);
+		hdr->hdr.csum_start = skb_checksum_start_offset(skb);
 		hdr->hdr.csum_offset = skb->csum_offset;
 	} else {
 		hdr->hdr.flags = 0;
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 65860a9..3b61e21 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -798,7 +798,7 @@ vmxnet3_parse_and_copy_hdr(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
 {
 	struct Vmxnet3_TxDataDesc *tdd;
 
-	if (ctx->mss) {
+	if (ctx->mss) {	/* GSO */
 		ctx->eth_ip_hdr_size = skb_transport_offset(skb);
 		ctx->l4_hdr_size = ((struct tcphdr *)
 				   skb_transport_header(skb))->doff * 4;
@@ -807,7 +807,7 @@ vmxnet3_parse_and_copy_hdr(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
 		unsigned int pull_size;
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
-			ctx->eth_ip_hdr_size = skb_transport_offset(skb);
+			ctx->eth_ip_hdr_size = skb_checksum_start_offset(skb);
 
 			if (ctx->ipv4) {
 				struct iphdr *iph = (struct iphdr *)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 19f37a6..0491da5 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1355,6 +1355,11 @@ static inline void skb_set_mac_header(struct sk_buff *skb, const int offset)
 }
 #endif /* NET_SKBUFF_DATA_USES_OFFSET */
 
+static inline int skb_checksum_start_offset(const struct sk_buff *skb)
+{
+	return skb->csum_start - skb_headroom(skb);
+}
+
 static inline int skb_transport_offset(const struct sk_buff *skb)
 {
 	return skb_transport_header(skb) - skb->data;


^ permalink raw reply related

* [PATCH] net: Fix too optimistic NETIF_F_HW_CSUM features
From: MichałMirosław @ 2010-11-30 14:23 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, Jesse Gross
In-Reply-To: <fddashfjkasdhklfjashlkjfs@rechot.qmqm.pl>

NETIF_F_HW_CSUM is superset of NETIF_F_IP_CSUM+NETIF_F_IPV6_CSUM, but
some drivers miss the difference. Fix this and also fix UFO dependency
on checksumming offload as it makes the same mistake in assumptions.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/benet/be_main.c           |    6 ++++--
 drivers/net/bnx2x/bnx2x_main.c        |    4 ++--
 drivers/net/jme.c                     |   16 +++++++++-------
 drivers/net/pch_gbe/pch_gbe_ethtool.c |   19 +------------------
 drivers/net/pch_gbe/pch_gbe_main.c    |    6 +++---
 drivers/net/sc92031.c                 |    3 ++-
 drivers/net/stmmac/stmmac_ethtool.c   |   12 +-----------
 drivers/net/stmmac/stmmac_main.c      |    5 +++--
 drivers/net/vxge/vxge-ethtool.c       |    2 +-
 drivers/net/vxge/vxge-main.c          |    2 +-
 net/core/dev.c                        |    7 +++++--
 net/core/ethtool.c                    |    4 +++-
 12 files changed, 35 insertions(+), 51 deletions(-)

diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 102567e..2754280 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -2583,10 +2583,12 @@ static void be_netdev_init(struct net_device *netdev)
 	int i;
 
 	netdev->features |= NETIF_F_SG | NETIF_F_HW_VLAN_RX | NETIF_F_TSO |
-		NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_FILTER | NETIF_F_HW_CSUM |
+		NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_FILTER |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 		NETIF_F_GRO | NETIF_F_TSO6;
 
-	netdev->vlan_features |= NETIF_F_SG | NETIF_F_TSO | NETIF_F_HW_CSUM;
+	netdev->vlan_features |= NETIF_F_SG | NETIF_F_TSO |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 
 	if (lancer_chip(adapter))
 		netdev->vlan_features |= NETIF_F_TSO6;
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index f53edfd..40ce95a 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -8761,7 +8761,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
 	dev->netdev_ops = &bnx2x_netdev_ops;
 	bnx2x_set_ethtool_ops(dev);
 	dev->features |= NETIF_F_SG;
-	dev->features |= NETIF_F_HW_CSUM;
+	dev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	if (bp->flags & USING_DAC_FLAG)
 		dev->features |= NETIF_F_HIGHDMA;
 	dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
@@ -8769,7 +8769,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
 	dev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
 
 	dev->vlan_features |= NETIF_F_SG;
-	dev->vlan_features |= NETIF_F_HW_CSUM;
+	dev->vlan_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	if (bp->flags & USING_DAC_FLAG)
 		dev->vlan_features |= NETIF_F_HIGHDMA;
 	dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index c57d9a4..ad0935c 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -2076,12 +2076,11 @@ jme_change_mtu(struct net_device *netdev, int new_mtu)
 	}
 
 	if (new_mtu > 1900) {
-		netdev->features &= ~(NETIF_F_HW_CSUM |
-				NETIF_F_TSO |
-				NETIF_F_TSO6);
+		netdev->features &= ~(NETIF_F_IP_CSUM | NETIFI_F_IPV6_CSUM |
+				NETIF_F_TSO | NETIF_F_TSO6);
 	} else {
 		if (test_bit(JME_FLAG_TXCSUM, &jme->flags))
-			netdev->features |= NETIF_F_HW_CSUM;
+			netdev->features |= NETIF_F_IP_CSUM | NETIFI_F_IPV6_CSUM;
 		if (test_bit(JME_FLAG_TSO, &jme->flags))
 			netdev->features |= NETIF_F_TSO | NETIF_F_TSO6;
 	}
@@ -2514,10 +2513,12 @@ jme_set_tx_csum(struct net_device *netdev, u32 on)
 	if (on) {
 		set_bit(JME_FLAG_TXCSUM, &jme->flags);
 		if (netdev->mtu <= 1900)
-			netdev->features |= NETIF_F_HW_CSUM;
+			netdev->features |=
+				NETIF_F_IP_CSUM | NETIFI_F_IPV6_CSUM;
 	} else {
 		clear_bit(JME_FLAG_TXCSUM, &jme->flags);
-		netdev->features &= ~NETIF_F_HW_CSUM;
+		netdev->features &=
+				~(NETIF_F_IP_CSUM | NETIFI_F_IPV6_CSUM);
 	}
 
 	return 0;
@@ -2797,7 +2798,8 @@ jme_init_one(struct pci_dev *pdev,
 	netdev->netdev_ops = &jme_netdev_ops;
 	netdev->ethtool_ops		= &jme_ethtool_ops;
 	netdev->watchdog_timeo		= TX_TIMEOUT;
-	netdev->features		=	NETIF_F_HW_CSUM |
+	netdev->features		=	NETIF_F_IP_CSUM |
+						NETIF_F_IPV6_CSUM |
 						NETIF_F_SG |
 						NETIF_F_TSO |
 						NETIF_F_TSO6 |
diff --git a/drivers/net/pch_gbe/pch_gbe_ethtool.c b/drivers/net/pch_gbe/pch_gbe_ethtool.c
index c8cc32c..c8c873b 100644
--- a/drivers/net/pch_gbe/pch_gbe_ethtool.c
+++ b/drivers/net/pch_gbe/pch_gbe_ethtool.c
@@ -469,18 +469,6 @@ static int pch_gbe_set_rx_csum(struct net_device *netdev, u32 data)
 }
 
 /**
- * pch_gbe_get_tx_csum - Report whether transmit checksums are turned on or off
- * @netdev:  Network interface device structure
- * Returns
- *	true(1):  Checksum On
- *	false(0): Checksum Off
- */
-static u32 pch_gbe_get_tx_csum(struct net_device *netdev)
-{
-	return (netdev->features & NETIF_F_HW_CSUM) != 0;
-}
-
-/**
  * pch_gbe_set_tx_csum - Turn transmit checksums on or off
  * @netdev: Network interface device structure
  * @data:   Checksum on[true] or off[false]
@@ -493,11 +481,7 @@ static int pch_gbe_set_tx_csum(struct net_device *netdev, u32 data)
 	struct pch_gbe_adapter *adapter = netdev_priv(netdev);
 
 	adapter->tx_csum = data;
-	if (data)
-		netdev->features |= NETIF_F_HW_CSUM;
-	else
-		netdev->features &= ~NETIF_F_HW_CSUM;
-	return 0;
+	return ethtool_op_set_tx_ipv6_csum(netdev, data);
 }
 
 /**
@@ -572,7 +556,6 @@ static const struct ethtool_ops pch_gbe_ethtool_ops = {
 	.set_pauseparam = pch_gbe_set_pauseparam,
 	.get_rx_csum = pch_gbe_get_rx_csum,
 	.set_rx_csum = pch_gbe_set_rx_csum,
-	.get_tx_csum = pch_gbe_get_tx_csum,
 	.set_tx_csum = pch_gbe_set_tx_csum,
 	.get_strings = pch_gbe_get_strings,
 	.get_ethtool_stats = pch_gbe_get_ethtool_stats,
diff --git a/drivers/net/pch_gbe/pch_gbe_main.c b/drivers/net/pch_gbe/pch_gbe_main.c
index afb7506..58e7903 100644
--- a/drivers/net/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/pch_gbe/pch_gbe_main.c
@@ -2319,7 +2319,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	netdev->watchdog_timeo = PCH_GBE_WATCHDOG_PERIOD;
 	netif_napi_add(netdev, &adapter->napi,
 		       pch_gbe_napi_poll, PCH_GBE_RX_WEIGHT);
-	netdev->features = NETIF_F_HW_CSUM | NETIF_F_GRO;
+	netdev->features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_GRO;
 	pch_gbe_set_ethtool_ops(netdev);
 
 	pch_gbe_mac_reset_hw(&adapter->hw);
@@ -2358,9 +2358,9 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	pch_gbe_check_options(adapter);
 
 	if (adapter->tx_csum)
-		netdev->features |= NETIF_F_HW_CSUM;
+		netdev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	else
-		netdev->features &= ~NETIF_F_HW_CSUM;
+		netdev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
 
 	/* initialize the wol settings based on the eeprom settings */
 	adapter->wake_up_evt = PCH_GBE_WL_INIT_SETTING;
diff --git a/drivers/net/sc92031.c b/drivers/net/sc92031.c
index 417adf3..76290a8 100644
--- a/drivers/net/sc92031.c
+++ b/drivers/net/sc92031.c
@@ -1449,7 +1449,8 @@ static int __devinit sc92031_probe(struct pci_dev *pdev,
 	dev->irq = pdev->irq;
 
 	/* faked with skb_copy_and_csum_dev */
-	dev->features = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA;
+	dev->features = NETIF_F_SG | NETIF_F_HIGHDMA |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 
 	dev->netdev_ops		= &sc92031_netdev_ops;
 	dev->watchdog_timeo	= TX_TIMEOUT;
diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index f2695fd..fd719ed 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -197,16 +197,6 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
 	}
 }
 
-static int stmmac_ethtool_set_tx_csum(struct net_device *netdev, u32 data)
-{
-	if (data)
-		netdev->features |= NETIF_F_HW_CSUM;
-	else
-		netdev->features &= ~NETIF_F_HW_CSUM;
-
-	return 0;
-}
-
 static u32 stmmac_ethtool_get_rx_csum(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
@@ -370,7 +360,7 @@ static struct ethtool_ops stmmac_ethtool_ops = {
 	.get_link = ethtool_op_get_link,
 	.get_rx_csum = stmmac_ethtool_get_rx_csum,
 	.get_tx_csum = ethtool_op_get_tx_csum,
-	.set_tx_csum = stmmac_ethtool_set_tx_csum,
+	.set_tx_csum = ethtool_op_set_tx_ipv6_csum,
 	.get_sg = ethtool_op_get_sg,
 	.set_sg = ethtool_op_set_sg,
 	.get_pauseparam = stmmac_get_pauseparam,
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 730a6fd..bfc2d12 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1494,7 +1494,8 @@ static int stmmac_probe(struct net_device *dev)
 	dev->netdev_ops = &stmmac_netdev_ops;
 	stmmac_set_ethtool_ops(dev);
 
-	dev->features |= (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA);
+	dev->features |= NETIF_F_SG | NETIF_F_HIGHDMA |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	dev->watchdog_timeo = msecs_to_jiffies(watchdog);
 #ifdef STMMAC_VLAN_TAG_USED
 	/* Both mac100 and gmac support receive VLAN tag detection */
@@ -1525,7 +1526,7 @@ static int stmmac_probe(struct net_device *dev)
 
 	DBG(probe, DEBUG, "%s: Scatter/Gather: %s - HW checksums: %s\n",
 	    dev->name, (dev->features & NETIF_F_SG) ? "on" : "off",
-	    (dev->features & NETIF_F_HW_CSUM) ? "on" : "off");
+	    (dev->features & NETIF_F_IP_CSUM) ? "on" : "off");
 
 	spin_lock_init(&priv->lock);
 
diff --git a/drivers/net/vxge/vxge-ethtool.c b/drivers/net/vxge/vxge-ethtool.c
index bc9bd10..5ece50d 100644
--- a/drivers/net/vxge/vxge-ethtool.c
+++ b/drivers/net/vxge/vxge-ethtool.c
@@ -1177,7 +1177,7 @@ static const struct ethtool_ops vxge_ethtool_ops = {
 	.get_rx_csum		= vxge_get_rx_csum,
 	.set_rx_csum		= vxge_set_rx_csum,
 	.get_tx_csum		= ethtool_op_get_tx_csum,
-	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
+	.set_tx_csum		= ethtool_op_set_tx_csum,
 	.get_sg			= ethtool_op_get_sg,
 	.set_sg			= ethtool_op_set_sg,
 	.get_tso		= ethtool_op_get_tso,
diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
index a21dae1..9f6d379 100644
--- a/drivers/net/vxge/vxge-main.c
+++ b/drivers/net/vxge/vxge-main.c
@@ -3368,7 +3368,7 @@ static int __devinit vxge_device_register(struct __vxge_hw_device *hldev,
 
 	ndev->features |= NETIF_F_SG;
 
-	ndev->features |= NETIF_F_HW_CSUM;
+	ndev->features |= NETIF_F_IP_CSUM;
 	vxge_debug_init(vxge_hw_device_trace_level_get(hldev),
 		"%s : checksuming enabled", __func__);
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 3259d2c..622f85a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5041,10 +5041,13 @@ unsigned long netdev_fix_features(unsigned long features, const char *name)
 	}
 
 	if (features & NETIF_F_UFO) {
-		if (!(features & NETIF_F_GEN_CSUM)) {
+		/* maybe split UFO into V4 and V6? */
+		if (!((features & NETIF_F_GEN_CSUM) ||
+		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
+			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
 			if (name)
 				printk(KERN_ERR "%s: Dropping NETIF_F_UFO "
-				       "since no NETIF_F_HW_CSUM feature.\n",
+				       "since no checksum offload features.\n",
 				       name);
 			features &= ~NETIF_F_UFO;
 		}
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 956a9f4..d5bc2881 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1171,7 +1171,9 @@ static int ethtool_set_ufo(struct net_device *dev, char __user *useraddr)
 		return -EFAULT;
 	if (edata.data && !(dev->features & NETIF_F_SG))
 		return -EINVAL;
-	if (edata.data && !(dev->features & NETIF_F_HW_CSUM))
+	if (edata.data && !((dev->features & NETIF_F_GEN_CSUM) ||
+		(dev->features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
+			== (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM)))
 		return -EINVAL;
 	return dev->ethtool_ops->set_ufo(dev, edata.data);
 }


^ permalink raw reply related

* Re: Broken TX checksumming offloads
From: MichałMirosław @ 2010-11-30 14:23 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, Jesse Gross
In-Reply-To: <AANLkTi=U51Y7rsQXr+_6TreUks_MyRMdRU=TtdxKOG2Z@mail.gmail.com>

On Mon, Nov 29, 2010 at 06:35:44PM -0800, Jesse Gross wrote:
> 2010/11/29 Ben Hutchings <bhutchings@solarflare.com>:
> > On Mon, 2010-11-29 at 19:17 +0100, Michał Mirosław wrote:
> >> Hi!
> >>
> >> Unless I'm horribly mistaken, generic HW checksumming works as follows:
> >>  - driver sets netdev->features & NETIF_F_HW_CSUM to indicate support
> >>    for generic checksumming; if the flag is not set, networking core
> >>    will checksum skb before calling ndo_start_xmit (let's ignore
> >>    other checksumming options for now) and not pass skb with
> >>    skb->ip_summed == CHECKSUM_PARTIAL
> >>  - ndo_start_xmit() should use skb->csum_start and skb->csum_offset
> >>    (or skb->csum) to update checksum in software or instruct HW to do so
> >>
> >> Looking at pch_gbe_xmit_frame() and its callee - pch_gbe_tx_queue() it
> >> looks like the driver should set NETIF_F_IP_CSUM instead of NETIF_F_HW_CSUM
> >> feature.
> >>
> >> Similar thing happens in ixgbe driver: it sets NETIF_F_HW_CSUM and checks
> >> for skb->ip_summed == CHECKSUM_PARTIAL, but then just warns on protocols
> >> other that TCP and SCTP (see: ixgbe_psum()).
> >
> > AFAIK only {TCP,UDP}/IPv{4,6} use the simple 16-bit checksum algorithm
> > that NETIF_F_HW_CSUM implies, so in practice it is equivalent to
> > NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM even though it doesn't mean the same
> > thing.
> 
> There are a few cases that I can think of where the extra generality
> of NETIF_F_HW_CSUM is useful:
> 
> * The checksum in GRE (maybe other protocols as well) uses the same
> algorithm as TCP/UDP.  In theory this means that we could use hardware
> offloading for GRE on cards that have NETIF_F_HW_CSUM.  The one issue
> is that NETIF_F_IP_CSUM really means TCP/UDP checksumming but we only
> test if the protocol is IP.  This means that other IP based protocols
> (like GRE) can't use checksum offloading since there is no software
> fallback path.
> 
> * TCP/UDP packets that are deeply encapsulated.  Currently we can
> express that checksum offloading is supported in one level of vlan
> encapsulation through vlan_features.  However, a NIC that exposes
> NETIF_F_HW_CSUM should in theory be able to checksum over a packet
> that has an arbitrary number of vlan tags in front of it, a GRE
> header, or any other type of encapsulation.

So the following patches should fix what's in tree regarding
NETIF_F_HW_CSUM interpretation. The patches have no interdependencies,
so can be applied independently. The last one is a cleanup that tagged
along.

Drivers that didn't care about csum_{start,offset}:

benet
bnx2x
jme
pch_gbe
sc92031
stmmac
vxge

Best Regards,
Michał Mirosław

^ permalink raw reply

* [PATCH] net: Move check of checksum features to netdev_fix_features()
From: MichałMirosław @ 2010-11-30 14:23 UTC (permalink / raw)
  To: netdev; +Cc: Ben Hutchings, Jesse Gross
In-Reply-To: <fddashfjkasdhklfjashlkjfs@rechot.qmqm.pl>

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

diff --git a/net/core/dev.c b/net/core/dev.c
index 622f85a..c64a848 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5023,6 +5023,23 @@ static void rollback_registered(struct net_device *dev)
 
 unsigned long netdev_fix_features(unsigned long features, const char *name)
 {
+	/* Fix illegal checksum combinations */
+	if ((features & NETIF_F_HW_CSUM) &&
+	    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
+		if (name)
+			printk(KERN_NOTICE "%s: mixed HW and IP checksum settings.\n",
+				name);
+		features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
+	}
+
+	if ((features & NETIF_F_NO_CSUM) &&
+	    (features & (NETIF_F_HW_CSUM|NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
+		if (name)
+			printk(KERN_NOTICE "%s: mixed no checksumming and other settings.\n",
+				name);
+		features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM|NETIF_F_HW_CSUM);
+	}
+
 	/* Fix illegal SG+CSUM combinations. */
 	if ((features & NETIF_F_SG) &&
 	    !(features & NETIF_F_ALL_CSUM)) {
@@ -5206,21 +5223,6 @@ int register_netdevice(struct net_device *dev)
 	if (dev->iflink == -1)
 		dev->iflink = dev->ifindex;
 
-	/* Fix illegal checksum combinations */
-	if ((dev->features & NETIF_F_HW_CSUM) &&
-	    (dev->features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
-		printk(KERN_NOTICE "%s: mixed HW and IP checksum settings.\n",
-		       dev->name);
-		dev->features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
-	}
-
-	if ((dev->features & NETIF_F_NO_CSUM) &&
-	    (dev->features & (NETIF_F_HW_CSUM|NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
-		printk(KERN_NOTICE "%s: mixed no checksumming and other settings.\n",
-		       dev->name);
-		dev->features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM|NETIF_F_HW_CSUM);
-	}
-
 	dev->features = netdev_fix_features(dev->features, dev->name);
 
 	/* Enable software GSO if SG is supported. */

^ permalink raw reply related

* Re: [PATCH] Net-ethtool : Allow ethtool to set interface in loopback mode.
From: Ben Hutchings @ 2010-11-30 15:01 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: David Miller, linux-netdev, laurent chavey
In-Reply-To: <AANLkTikVaVv4fkLom+NSQgOUzCd2i3mbHkBNQ6PD4d9Y@mail.gmail.com>

On Tue, 2010-11-30 at 00:00 -0800, Mahesh Bandewar wrote:
> This patch enables ethtool to set the loopback mode on a given
> interface. This is the reworked version of earlier submit (which I
> don't have reference to). By configuring the interface in loopback
> mode in conjunction with a policy route / rule, a userland application
> can stress the egress / ingress path exposing the flows of the change
> in progress and potentially help developer(s) understand the impact of
> those changes without even sending a packet out on the network.

Is the aim to stress the generic egress/ingress code or also to cover
the specific driver in use?

I note that your colleague Laurent Chavey posted a very similar patch
back in April <http://article.gmane.org/gmane.linux.network/157489> but
he emphasised hardware diagnosis.

> Following set of commands illustrates one such example -
>     a) ifconfig eth1 192.168.1.1
>     b) ip -4 rule add from all iif eth1 lookup 250
>     c) ip -4 route add local 0/0 dev lo proto kernel scope host table 250
>     d) arp -Ds 192.168.1.100 eth1
>     e) arp -Ds 192.168.1.200 eth1
>     f) sysctl -w net.ipv4.ip_nonlocal_bind=1
>     g) sysctl -w net.ipv4.conf.all.accept_local=1
>     # Assuming that the machine has 8 cores
>     h) taskset 000f netserver -L 192.168.1.200
>     i) taskset 00f0 netperf -t TCP_CRR -L 192.168.1.100 -H 192.168.1.200 -l 30
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  include/linux/ethtool.h |    4 ++++
>  net/core/ethtool.c      |   39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 6628a50..7523d45 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -678,6 +678,8 @@ struct ethtool_ops {
>                                   struct ethtool_rxfh_indir *);
>         int     (*set_rxfh_indir)(struct net_device *,
>                                   const struct ethtool_rxfh_indir *);
> +       int     (*get_loopback)(struct net_device *, u32 *);
> +       int     (*set_loopback)(struct net_device *, u32);
>  };
>  #endif /* __KERNEL__ */
> 
> @@ -741,6 +743,8 @@ struct ethtool_ops {
>  #define ETHTOOL_GSSET_INFO     0x00000037 /* Get string set info */
>  #define ETHTOOL_GRXFHINDIR     0x00000038 /* Get RX flow hash indir'n table */
>  #define ETHTOOL_SRXFHINDIR     0x00000039 /* Set RX flow hash indir'n table */
> +#define ETHTOOL_SLOOPBACK      0x0000003a /* Enable / Disable Loopback */
[...]

Where should loopback be done, when enabled?  As near as possible to the
host, so it only covers the DMA engines, or as far away as possible, so
it covers most of the MAC/PHY hardware?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH] net: Fix too optimistic NETIF_F_HW_CSUM features
From: MichałMirosław @ 2010-11-30 15:28 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Jesse Gross
In-Reply-To: <1291130005.21077.18.camel@bwh-desktop>

On Tue, Nov 30, 2010 at 03:13:25PM +0000, Ben Hutchings wrote:
> On Tue, 2010-11-30 at 15:23 +0100, MichałMirosław wrote:
> > NETIF_F_HW_CSUM is superset of NETIF_F_IP_CSUM+NETIF_F_IPV6_CSUM, but
> > some drivers miss the difference. Fix this and also fix UFO dependency
> > on checksumming offload as it makes the same mistake in assumptions.
> [...]
> > diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
> > index f53edfd..40ce95a 100644
> > --- a/drivers/net/bnx2x/bnx2x_main.c
> > +++ b/drivers/net/bnx2x/bnx2x_main.c
> > @@ -8761,7 +8761,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
> >  	dev->netdev_ops = &bnx2x_netdev_ops;
> >  	bnx2x_set_ethtool_ops(dev);
> >  	dev->features |= NETIF_F_SG;
> > -	dev->features |= NETIF_F_HW_CSUM;
> > +	dev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> >  	if (bp->flags & USING_DAC_FLAG)
> >  		dev->features |= NETIF_F_HIGHDMA;
> >  	dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> > @@ -8769,7 +8769,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
> >  	dev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
> >  
> >  	dev->vlan_features |= NETIF_F_SG;
> > -	dev->vlan_features |= NETIF_F_HW_CSUM;
> > +	dev->vlan_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> >  	if (bp->flags & USING_DAC_FLAG)
> >  		dev->vlan_features |= NETIF_F_HIGHDMA;
> >  	dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> > diff --git a/drivers/net/jme.c b/drivers/net/jme.c
> > index c57d9a4..ad0935c 100644
> > --- a/drivers/net/jme.c
> > +++ b/drivers/net/jme.c
> > @@ -2076,12 +2076,11 @@ jme_change_mtu(struct net_device *netdev, int new_mtu)
> >  	}
> >  
> >  	if (new_mtu > 1900) {
> > -		netdev->features &= ~(NETIF_F_HW_CSUM |
> > -				NETIF_F_TSO |
> > -				NETIF_F_TSO6);
> > +		netdev->features &= ~(NETIF_F_IP_CSUM | NETIFI_F_IPV6_CSUM |
> > +				NETIF_F_TSO | NETIF_F_TSO6);
> >  	} else {
> >  		if (test_bit(JME_FLAG_TXCSUM, &jme->flags))
> > -			netdev->features |= NETIF_F_HW_CSUM;
> > +			netdev->features |= NETIF_F_IP_CSUM | NETIFI_F_IPV6_CSUM;
> 
> In this file you've written 'NETIFI_F_IPV6_CSUM' several times.  Do try
> compiling your work. :-)

Sorry for that. I compile tested whole series, where another patch I'm working on
just removes this part.

> [...]
> > diff --git a/drivers/net/pch_gbe/pch_gbe_ethtool.c b/drivers/net/pch_gbe/pch_gbe_ethtool.c
> > index c8cc32c..c8c873b 100644
> > --- a/drivers/net/pch_gbe/pch_gbe_ethtool.c
> > +++ b/drivers/net/pch_gbe/pch_gbe_ethtool.c
> > @@ -469,18 +469,6 @@ static int pch_gbe_set_rx_csum(struct net_device *netdev, u32 data)
> >  }
> >  
> >  /**
> > - * pch_gbe_get_tx_csum - Report whether transmit checksums are turned on or off
> > - * @netdev:  Network interface device structure
> > - * Returns
> > - *	true(1):  Checksum On
> > - *	false(0): Checksum Off
> > - */
> > -static u32 pch_gbe_get_tx_csum(struct net_device *netdev)
> > -{
> > -	return (netdev->features & NETIF_F_HW_CSUM) != 0;
> > -}
> > -
> > -/**
> >   * pch_gbe_set_tx_csum - Turn transmit checksums on or off
> >   * @netdev: Network interface device structure
> >   * @data:   Checksum on[true] or off[false]
> > @@ -493,11 +481,7 @@ static int pch_gbe_set_tx_csum(struct net_device *netdev, u32 data)
> >  	struct pch_gbe_adapter *adapter = netdev_priv(netdev);
> >  
> >  	adapter->tx_csum = data;
> > -	if (data)
> > -		netdev->features |= NETIF_F_HW_CSUM;
> > -	else
> > -		netdev->features &= ~NETIF_F_HW_CSUM;
> > -	return 0;
> > +	return ethtool_op_set_tx_ipv6_csum(netdev, data);
> >  }
> >  
> >  /**
> > @@ -572,7 +556,6 @@ static const struct ethtool_ops pch_gbe_ethtool_ops = {
> >  	.set_pauseparam = pch_gbe_set_pauseparam,
> >  	.get_rx_csum = pch_gbe_get_rx_csum,
> >  	.set_rx_csum = pch_gbe_set_rx_csum,
> > -	.get_tx_csum = pch_gbe_get_tx_csum,
> >  	.set_tx_csum = pch_gbe_set_tx_csum,
> >  	.get_strings = pch_gbe_get_strings,
> >  	.get_ethtool_stats = pch_gbe_get_ethtool_stats,
> 
> pch_gbe_get_tx_csum can simply be replaced with
> ethtool_op_set_tx_ipv6_csum.

pch_gbe_set_tx_csum() also changes adapter->tx_csum, which I didn't want
to touch in this patch. (I'm assuming you mean ...set_tx_csum not ...get_tx_csum).

> [...]
> > diff --git a/drivers/net/vxge/vxge-ethtool.c b/drivers/net/vxge/vxge-ethtool.c
> > index bc9bd10..5ece50d 100644
> > --- a/drivers/net/vxge/vxge-ethtool.c
> > +++ b/drivers/net/vxge/vxge-ethtool.c
> > @@ -1177,7 +1177,7 @@ static const struct ethtool_ops vxge_ethtool_ops = {
> >  	.get_rx_csum		= vxge_get_rx_csum,
> >  	.set_rx_csum		= vxge_set_rx_csum,
> >  	.get_tx_csum		= ethtool_op_get_tx_csum,
> > -	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
> > +	.set_tx_csum		= ethtool_op_set_tx_csum,
> >  	.get_sg			= ethtool_op_get_sg,
> >  	.set_sg			= ethtool_op_set_sg,
> >  	.get_tso		= ethtool_op_get_tso,
> > diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> > index a21dae1..9f6d379 100644
> > --- a/drivers/net/vxge/vxge-main.c
> > +++ b/drivers/net/vxge/vxge-main.c
> > @@ -3368,7 +3368,7 @@ static int __devinit vxge_device_register(struct __vxge_hw_device *hldev,
> >  
> >  	ndev->features |= NETIF_F_SG;
> >  
> > -	ndev->features |= NETIF_F_HW_CSUM;
> > +	ndev->features |= NETIF_F_IP_CSUM;
> >  	vxge_debug_init(vxge_hw_device_trace_level_get(hldev),
> >  		"%s : checksuming enabled", __func__);
> >  
> Why are you disabling IPv6 checksum offload?  From a quick look at the
> driver, I think the hardware does support it.

In vxge_xmit() (at drivers/net/vxge/vxge-main.c:922 in net-next) there
is the following code, that suggested otherwise:

        if (skb->ip_summed == CHECKSUM_PARTIAL)
                vxge_hw_fifo_txdl_cksum_set_bits(dtr,
                                        VXGE_HW_FIFO_TXD_TX_CKO_IPV4_EN |
                                        VXGE_HW_FIFO_TXD_TX_CKO_TCP_EN |
                                        VXGE_HW_FIFO_TXD_TX_CKO_UDP_EN);

> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 3259d2c..622f85a 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -5041,10 +5041,13 @@ unsigned long netdev_fix_features(unsigned long features, const char *name)
> >  	}
> >  
> >  	if (features & NETIF_F_UFO) {
> > -		if (!(features & NETIF_F_GEN_CSUM)) {
> > +		/* maybe split UFO into V4 and V6? */
> > +		if (!((features & NETIF_F_GEN_CSUM) ||
> > +		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
> > +			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
> >  			if (name)
> >  				printk(KERN_ERR "%s: Dropping NETIF_F_UFO "
> > -				       "since no NETIF_F_HW_CSUM feature.\n",
> > +				       "since no checksum offload features.\n",
> >  				       name);
> >  			features &= ~NETIF_F_UFO;
> >  		}
> > diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> > index 956a9f4..d5bc2881 100644
> > --- a/net/core/ethtool.c
> > +++ b/net/core/ethtool.c
> > @@ -1171,7 +1171,9 @@ static int ethtool_set_ufo(struct net_device *dev, char __user *useraddr)
> >  		return -EFAULT;
> >  	if (edata.data && !(dev->features & NETIF_F_SG))
> >  		return -EINVAL;
> > -	if (edata.data && !(dev->features & NETIF_F_HW_CSUM))
> > +	if (edata.data && !((dev->features & NETIF_F_GEN_CSUM) ||
> > +		(dev->features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
> > +			== (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM)))
> >  		return -EINVAL;
> >  	return dev->ethtool_ops->set_ufo(dev, edata.data);
> >  }
> I believe UFO is for IPv4 only; IPv6 has an entirely different kind of
> fragmentation.  So I think the check should be dev->features &
> NETIF_F_V4_CSUM.

net/ipv6/ip6_output.c references NETIF_F_UFO without checking
IPv6 checksumming features.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH] net: Fix too optimistic NETIF_F_HW_CSUM features
From: Michał Mirosław @ 2010-11-30 15:41 UTC (permalink / raw)
  To: Ramkrishna Vepa, Sivakumar Subramani, Sreenivasa Honnur,
	Jon Mason
  Cc: netdev, Ben Hutchings, Jesse Gross
In-Reply-To: <20101130152838.GA26281@rere.qmqm.pl>

On Tue, Nov 30, 2010 at 04:28:38PM +0100, MichałMirosław wrote:
> On Tue, Nov 30, 2010 at 03:13:25PM +0000, Ben Hutchings wrote:
> > On Tue, 2010-11-30 at 15:23 +0100, MichałMirosław wrote:
> > > NETIF_F_HW_CSUM is superset of NETIF_F_IP_CSUM+NETIF_F_IPV6_CSUM, but
> > > some drivers miss the difference. Fix this and also fix UFO dependency
> > > on checksumming offload as it makes the same mistake in assumptions.
[...]
> > > diff --git a/drivers/net/vxge/vxge-ethtool.c b/drivers/net/vxge/vxge-ethtool.c
> > > index bc9bd10..5ece50d 100644
> > > --- a/drivers/net/vxge/vxge-ethtool.c
> > > +++ b/drivers/net/vxge/vxge-ethtool.c
> > > @@ -1177,7 +1177,7 @@ static const struct ethtool_ops vxge_ethtool_ops = {
> > >  	.get_rx_csum		= vxge_get_rx_csum,
> > >  	.set_rx_csum		= vxge_set_rx_csum,
> > >  	.get_tx_csum		= ethtool_op_get_tx_csum,
> > > -	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
> > > +	.set_tx_csum		= ethtool_op_set_tx_csum,
> > >  	.get_sg			= ethtool_op_get_sg,
> > >  	.set_sg			= ethtool_op_set_sg,
> > >  	.get_tso		= ethtool_op_get_tso,
> > > diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> > > index a21dae1..9f6d379 100644
> > > --- a/drivers/net/vxge/vxge-main.c
> > > +++ b/drivers/net/vxge/vxge-main.c
> > > @@ -3368,7 +3368,7 @@ static int __devinit vxge_device_register(struct __vxge_hw_device *hldev,
> > >  
> > >  	ndev->features |= NETIF_F_SG;
> > >  
> > > -	ndev->features |= NETIF_F_HW_CSUM;
> > > +	ndev->features |= NETIF_F_IP_CSUM;
> > >  	vxge_debug_init(vxge_hw_device_trace_level_get(hldev),
> > >  		"%s : checksuming enabled", __func__);
> > >  
> > Why are you disabling IPv6 checksum offload?  From a quick look at the
> > driver, I think the hardware does support it.
> 
> In vxge_xmit() (at drivers/net/vxge/vxge-main.c:922 in net-next) there
> is the following code, that suggested otherwise:
> 
>         if (skb->ip_summed == CHECKSUM_PARTIAL)
>                 vxge_hw_fifo_txdl_cksum_set_bits(dtr,
>                                         VXGE_HW_FIFO_TXD_TX_CKO_IPV4_EN |
>                                         VXGE_HW_FIFO_TXD_TX_CKO_TCP_EN |
>                                         VXGE_HW_FIFO_TXD_TX_CKO_UDP_EN);
> 

Lets ask vxge driver maintainters on this.

Does vxge support IPv6 TCP/UDP checksumming offload?

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: Bonding, GRO and tcp_reordering
From: Ben Hutchings @ 2010-11-30 15:42 UTC (permalink / raw)
  To: Simon Horman; +Cc: netdev
In-Reply-To: <20101130135549.GA22688@verge.net.au>

On Tue, 2010-11-30 at 22:55 +0900, Simon Horman wrote:
> Hi,
> 
> I just wanted to share what is a rather pleasing,
> though to me somewhat surprising result.
>
> I am testing bonding using balance-rr mode with three physical links to try
> to get > gigabit speed for a single stream. Why?  Because I'd like to run
> various tests at > gigabit speed and I don't have any 10G hardware at my
> disposal.
> 
> The result I have is that with a 1500 byte MTU, tcp_reordering=3 and both
> LSO and GSO disabled on both the sender and receiver I see:
> 
> # netperf -c -4 -t TCP_STREAM -H 172.17.60.216 -- -m 1472
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216
> (172.17.60.216) port 0 AF_INET
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB
> 
>   87380  16384   1472    10.01      1646.13   40.01    -1.00    3.982  -1.000
> 
> But with GRO enabled on the receiver I see.
> 
> # netperf -c -4 -t TCP_STREAM -H 172.17.60.216 -- -m 1472
> TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 172.17.60.216
> (172.17.60.216) port 0 AF_INET
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB
> 
>  87380  16384   1472    10.01      2613.83   19.32    -1.00    1.211   -1.000
> 
> Which is much better than any result I get tweaking tcp_reordering when
> GRO is disabled on the receiver.

Did you also enable TSO/GSO on the sender?

What TSO/GSO will do is to change the round-robin scheduling from one
packet per interface to one super-packet per interface.  GRO then
coalesces the physical packets back into a super-packet.  The intervals
between receiving super-packets then tend to exceed the difference in
delay between interfaces, hiding the reordering.

If you only enabled GRO then I don't understand this.

> Tweaking tcp_reordering when GRO is enabled on the receiver seems to have
> negligible effect.  Which is interesting, because my brief reading on the
> subject indicated that tcp_reordering was the key tuning parameter for
> bonding with balance-rr.
> 
> The only other parameter that seemed to have significant effect was to
> increase the mtu.  In the case of MTU=9000, GRO seemed to have a negative
> impact on throughput, though a significant positive effect on CPU
> utilisation.
[...]

Increasing MTU also increases the interval between packets on a TCP flow
using maximum segment size so that it is more likely to exceed the
difference in delay.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* [PATCH 0/5] xfrm: ESP Traffic Flow Confidentiality padding
From: Martin Willi @ 2010-11-30 15:49 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, netdev

The following patchset adds Traffic Flow Confidentiality padding. The
first patch introduces a new Netlink XFRM attribute to configure TFC via
userspace. The second patch removes an existing padlen option in ESP; It
is not used at all, and I currently don't see the purpose of the field,
nor how it should interact with TFC padding enabled. Patch three and four
implement the padding logic in IPv4 and IPv6 ESP.

Padding is specified with a length to pad the encapsulated data to.
Support for TFC padding as specified in RFC4303 must be negotiated
explicitly by the key management protocol, hence the optional flag. The
fallback with ESP padding field expansion is limited to 255 padding
bytes. If this is insufficient, padding length is randomized to hide
the real length as good as possible.

The last patch adds an option to pad all packets to the PMTU. It works
fine for simple scenarios, but I'm not sure if my PMTU lookup works in
all cases (nested transforms?). Any pointer would be appreciated.

Martin Willi (5):
      xfrm: Add Traffic Flow Confidentiality padding XFRM attribute
      xfrm: Remove unused ESP padlen field
      xfrm: Traffic Flow Confidentiality for IPv4 ESP
      xfrm: Traffic Flow Confidentiality for IPv6 ESP
      xfrm: Add TFC padding option to automatically pad to PMTU

 include/linux/xfrm.h |    8 +++++++
 include/net/esp.h    |    3 --
 include/net/xfrm.h   |    1 +
 net/ipv4/esp4.c      |   58 +++++++++++++++++++++++++++++++++++--------------
 net/ipv6/esp6.c      |   58 +++++++++++++++++++++++++++++++++++--------------
 net/xfrm/xfrm_user.c |   16 ++++++++++++-
 6 files changed, 105 insertions(+), 39 deletions(-)

^ permalink raw reply

* [PATCH 1/5] xfrm: Add Traffic Flow Confidentiality padding XFRM attribute
From: Martin Willi @ 2010-11-30 15:49 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, netdev
In-Reply-To: <1291132155-31277-1-git-send-email-martin@strongswan.org>

The XFRMA_TFCPAD attribute for XFRM state installation configures
Traffic Flow Confidentiality by padding ESP packets to a specified
length. To use RFC4303 TFC padding and overcome the 255 byte ESP
padding field limit, the XFRM_TFC_ESPV3 flag must be set.

Signed-off-by: Martin Willi <martin@strongswan.org>
---
 include/linux/xfrm.h |    7 +++++++
 include/net/xfrm.h   |    1 +
 net/xfrm/xfrm_user.c |   16 ++++++++++++++--
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index b971e38..b1e5f8a 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -283,6 +283,7 @@ enum xfrm_attr_type_t {
 	XFRMA_KMADDRESS,        /* struct xfrm_user_kmaddress */
 	XFRMA_ALG_AUTH_TRUNC,	/* struct xfrm_algo_auth */
 	XFRMA_MARK,		/* struct xfrm_mark */
+	XFRMA_TFC,		/* struct xfrm_tfc */
 	__XFRMA_MAX
 
 #define XFRMA_MAX (__XFRMA_MAX - 1)
@@ -293,6 +294,12 @@ struct xfrm_mark {
 	__u32           m; /* mask */
 };
 
+struct xfrm_tfc {
+	__u16		pad;
+	__u16		flags;
+#define XFRM_TFC_ESPV3	1	/* RFC4303 TFC padding, if possible */
+};
+
 enum xfrm_sadattr_type_t {
 	XFRMA_SAD_UNSPEC,
 	XFRMA_SAD_CNT,
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index bcfb6b2..03468c0 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -143,6 +143,7 @@ struct xfrm_state {
 	struct xfrm_id		id;
 	struct xfrm_selector	sel;
 	struct xfrm_mark	mark;
+	struct xfrm_tfc		tfc;
 
 	u32			genid;
 
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 8bae6b2..0b4ec02 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -148,7 +148,8 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
 		     !attrs[XFRMA_ALG_AUTH_TRUNC]) ||
 		    attrs[XFRMA_ALG_AEAD]	||
 		    attrs[XFRMA_ALG_CRYPT]	||
-		    attrs[XFRMA_ALG_COMP])
+		    attrs[XFRMA_ALG_COMP]	||
+		    attrs[XFRMA_TFC])
 			goto out;
 		break;
 
@@ -172,7 +173,8 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
 		    attrs[XFRMA_ALG_AEAD]	||
 		    attrs[XFRMA_ALG_AUTH]	||
 		    attrs[XFRMA_ALG_AUTH_TRUNC]	||
-		    attrs[XFRMA_ALG_CRYPT])
+		    attrs[XFRMA_ALG_CRYPT]	||
+		    attrs[XFRMA_TFC])
 			goto out;
 		break;
 
@@ -186,6 +188,7 @@ static int verify_newsa_info(struct xfrm_usersa_info *p,
 		    attrs[XFRMA_ALG_CRYPT]	||
 		    attrs[XFRMA_ENCAP]		||
 		    attrs[XFRMA_SEC_CTX]	||
+		    attrs[XFRMA_TFC]		||
 		    !attrs[XFRMA_COADDR])
 			goto out;
 		break;
@@ -439,6 +442,9 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
 			goto error;
 	}
 
+	if (attrs[XFRMA_TFC])
+		memcpy(&x->tfc, nla_data(attrs[XFRMA_TFC]), sizeof(x->tfc));
+
 	if (attrs[XFRMA_COADDR]) {
 		x->coaddr = kmemdup(nla_data(attrs[XFRMA_COADDR]),
 				    sizeof(*x->coaddr), GFP_KERNEL);
@@ -688,6 +694,9 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
 	if (x->encap)
 		NLA_PUT(skb, XFRMA_ENCAP, sizeof(*x->encap), x->encap);
 
+	if (x->tfc.pad || x->tfc.flags)
+		NLA_PUT(skb, XFRMA_TFC, sizeof(x->tfc), &x->tfc);
+
 	if (xfrm_mark_put(skb, &x->mark))
 		goto nla_put_failure;
 
@@ -2122,6 +2131,7 @@ static const struct nla_policy xfrma_policy[XFRMA_MAX+1] = {
 	[XFRMA_MIGRATE]		= { .len = sizeof(struct xfrm_user_migrate) },
 	[XFRMA_KMADDRESS]	= { .len = sizeof(struct xfrm_user_kmaddress) },
 	[XFRMA_MARK]		= { .len = sizeof(struct xfrm_mark) },
+	[XFRMA_TFC]		= { .len = sizeof(struct xfrm_tfc) },
 };
 
 static struct xfrm_link {
@@ -2301,6 +2311,8 @@ static inline size_t xfrm_sa_len(struct xfrm_state *x)
 		l += nla_total_size(sizeof(*x->calg));
 	if (x->encap)
 		l += nla_total_size(sizeof(*x->encap));
+	if (x->tfc.pad)
+		l += nla_total_size(sizeof(x->tfc));
 	if (x->security)
 		l += nla_total_size(sizeof(struct xfrm_user_sec_ctx) +
 				    x->security->ctx_len);
-- 
1.7.1

^ permalink raw reply related

* [PATCH 3/5] xfrm: Traffic Flow Confidentiality for IPv4 ESP
From: Martin Willi @ 2010-11-30 15:49 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, netdev
In-Reply-To: <1291132155-31277-1-git-send-email-martin@strongswan.org>

If configured on xfrm state, increase the length of all packets to
a given boundary using TFC padding as specified in RFC4303. For
transport mode, or if the XFRM_TFC_ESPV3 is not set, grow the ESP
padding field instead.

Signed-off-by: Martin Willi <martin@strongswan.org>
---
 net/ipv4/esp4.c |   42 +++++++++++++++++++++++++++++++++---------
 1 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 67e4c12..a6adfbc 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -117,23 +117,43 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 	int blksize;
 	int clen;
 	int alen;
+	int plen;
+	int tfclen;
+	int tfcpadto;
 	int nfrags;
 
 	/* skb is pure payload to encrypt */
 
 	err = -ENOMEM;
 
-	/* Round to block size */
-	clen = skb->len;
-
 	esp = x->data;
 	aead = esp->aead;
 	alen = crypto_aead_authsize(aead);
 
 	blksize = ALIGN(crypto_aead_blocksize(aead), 4);
-	clen = ALIGN(clen + 2, blksize);
-
-	if ((err = skb_cow_data(skb, clen - skb->len + alen, &trailer)) < 0)
+	tfclen = 0;
+	tfcpadto = x->tfc.pad;
+
+	if (skb->len >= tfcpadto) {
+		clen = ALIGN(skb->len + 2, blksize);
+	} else if (x->tfc.flags & XFRM_TFC_ESPV3 &&
+		   x->props.mode == XFRM_MODE_TUNNEL) {
+		/* ESPv3 TFC padding, append bytes to payload */
+		tfclen = tfcpadto - skb->len;
+		clen = ALIGN(skb->len + 2 + tfclen, blksize);
+	} else {
+		/* ESPv2 TFC padding. If we exceed the 255 byte maximum, use
+		 * random padding to hide payload length as good as possible. */
+		clen = ALIGN(skb->len + 2 + tfcpadto - skb->len, blksize);
+		if (clen - skb->len - 2 > 255) {
+			clen = ALIGN(skb->len + (u8)random32() + 2, blksize);
+			if (clen - skb->len - 2 > 255)
+				clen -= blksize;
+		}
+	}
+	plen = clen - skb->len - tfclen;
+	err = skb_cow_data(skb, tfclen + plen + alen, &trailer);
+	if (err < 0)
 		goto error;
 	nfrags = err;
 
@@ -148,13 +168,17 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 
 	/* Fill padding... */
 	tail = skb_tail_pointer(trailer);
+	if (tfclen) {
+		memset(tail, 0, tfclen);
+		tail += tfclen;
+	}
 	do {
 		int i;
-		for (i=0; i<clen-skb->len - 2; i++)
+		for (i = 0; i < plen - 2; i++)
 			tail[i] = i + 1;
 	} while (0);
-	tail[clen - skb->len - 2] = (clen - skb->len) - 2;
-	tail[clen - skb->len - 1] = *skb_mac_header(skb);
+	tail[plen - 2] = plen - 2;
+	tail[plen - 1] = *skb_mac_header(skb);
 	pskb_put(skb, trailer, clen - skb->len + alen);
 
 	skb_push(skb, -skb_network_offset(skb));
-- 
1.7.1

^ permalink raw reply related

* [PATCH 5/5] xfrm: Add TFC padding option to automatically pad to PMTU
From: Martin Willi @ 2010-11-30 15:49 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, netdev
In-Reply-To: <1291132155-31277-1-git-send-email-martin@strongswan.org>

Traffic Flow Confidentiality padding is most effective if all packets
have exactly the same size. For SAs with mixed traffic, the largest
packet size is usually the PMTU. Instead of calculating the PMTU
manually, the XFRM_TFC_PMTU flag automatically pads to the PMTU.

Signed-off-by: Martin Willi <martin@strongswan.org>
---
 include/linux/xfrm.h |    1 +
 net/ipv4/esp4.c      |    7 +++++++
 net/ipv6/esp6.c      |    7 +++++++
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index b1e5f8a..2a9f0b4 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -298,6 +298,7 @@ struct xfrm_tfc {
 	__u16		pad;
 	__u16		flags;
 #define XFRM_TFC_ESPV3	1	/* RFC4303 TFC padding, if possible */
+#define XFRM_TFC_PMTU	2	/* ignore pad field, pad to PMTU */
 };
 
 enum xfrm_sadattr_type_t {
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index a6adfbc..cfb4992 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -23,6 +23,8 @@ struct esp_skb_cb {
 
 #define ESP_SKB_CB(__skb) ((struct esp_skb_cb *)&((__skb)->cb[0]))
 
+static u32 esp4_get_mtu(struct xfrm_state *x, int mtu);
+
 /*
  * Allocate an AEAD request structure with extra space for SG and IV.
  *
@@ -133,6 +135,11 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 	blksize = ALIGN(crypto_aead_blocksize(aead), 4);
 	tfclen = 0;
 	tfcpadto = x->tfc.pad;
+	if (x->tfc.flags & XFRM_TFC_PMTU) {
+		struct xfrm_dst *dst = (struct xfrm_dst *)skb_dst(skb);
+
+		tfcpadto = esp4_get_mtu(x, dst->child_mtu_cached);
+	}
 
 	if (skb->len >= tfcpadto) {
 		clen = ALIGN(skb->len + 2, blksize);
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 9494cb1..6cb9a02 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -49,6 +49,8 @@ struct esp_skb_cb {
 
 #define ESP_SKB_CB(__skb) ((struct esp_skb_cb *)&((__skb)->cb[0]))
 
+static u32 esp6_get_mtu(struct xfrm_state *x, int mtu);
+
 /*
  * Allocate an AEAD request structure with extra space for SG and IV.
  *
@@ -157,6 +159,11 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 	blksize = ALIGN(crypto_aead_blocksize(aead), 4);
 	tfclen = 0;
 	tfcpadto = x->tfc.pad;
+	if (x->tfc.flags & XFRM_TFC_PMTU) {
+		struct xfrm_dst *dst = (struct xfrm_dst *)skb_dst(skb);
+
+		tfcpadto = esp6_get_mtu(x, dst->child_mtu_cached);
+	}
 
 	if (skb->len >= tfcpadto) {
 		clen = ALIGN(skb->len + 2, blksize);
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] net: Fix too optimistic NETIF_F_HW_CSUM features
From: Ben Hutchings @ 2010-11-30 15:51 UTC (permalink / raw)
  To: MichałMirosław; +Cc: netdev, Jesse Gross
In-Reply-To: <20101130152838.GA26281@rere.qmqm.pl>

On Tue, 2010-11-30 at 16:28 +0100, MichałMirosław wrote:
[...]
> > > -/**
> > >   * pch_gbe_set_tx_csum - Turn transmit checksums on or off
> > >   * @netdev: Network interface device structure
> > >   * @data:   Checksum on[true] or off[false]
> > > @@ -493,11 +481,7 @@ static int pch_gbe_set_tx_csum(struct net_device *netdev, u32 data)
> > >  	struct pch_gbe_adapter *adapter = netdev_priv(netdev);
> > >  
> > >  	adapter->tx_csum = data;
> > > -	if (data)
> > > -		netdev->features |= NETIF_F_HW_CSUM;
> > > -	else
> > > -		netdev->features &= ~NETIF_F_HW_CSUM;
> > > -	return 0;
> > > +	return ethtool_op_set_tx_ipv6_csum(netdev, data);
> > >  }
> > >  
> > >  /**
> > > @@ -572,7 +556,6 @@ static const struct ethtool_ops pch_gbe_ethtool_ops = {
> > >  	.set_pauseparam = pch_gbe_set_pauseparam,
> > >  	.get_rx_csum = pch_gbe_get_rx_csum,
> > >  	.set_rx_csum = pch_gbe_set_rx_csum,
> > > -	.get_tx_csum = pch_gbe_get_tx_csum,
> > >  	.set_tx_csum = pch_gbe_set_tx_csum,
> > >  	.get_strings = pch_gbe_get_strings,
> > >  	.get_ethtool_stats = pch_gbe_get_ethtool_stats,
> > 
> > pch_gbe_get_tx_csum can simply be replaced with
> > ethtool_op_set_tx_ipv6_csum.
> 
> pch_gbe_set_tx_csum() also changes adapter->tx_csum, which I didn't want
> to touch in this patch. (I'm assuming you mean ...set_tx_csum not ...get_tx_csum).

Sorry, I missed that.

[...]
> > Why are you disabling IPv6 checksum offload?  From a quick look at the
> > driver, I think the hardware does support it.
> 
> In vxge_xmit() (at drivers/net/vxge/vxge-main.c:922 in net-next) there
> is the following code, that suggested otherwise:
> 
>         if (skb->ip_summed == CHECKSUM_PARTIAL)
>                 vxge_hw_fifo_txdl_cksum_set_bits(dtr,
>                                         VXGE_HW_FIFO_TXD_TX_CKO_IPV4_EN |
>                                         VXGE_HW_FIFO_TXD_TX_CKO_TCP_EN |
>                                         VXGE_HW_FIFO_TXD_TX_CKO_UDP_EN);

I bet IPV4_EN refers to the IPv4 header checksum.  Since IPv6 doesn't
have a header checksum, there won't be a flag for it here.

> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 3259d2c..622f85a 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -5041,10 +5041,13 @@ unsigned long netdev_fix_features(unsigned long features, const char *name)
> > >  	}
> > >  
> > >  	if (features & NETIF_F_UFO) {
> > > -		if (!(features & NETIF_F_GEN_CSUM)) {
> > > +		/* maybe split UFO into V4 and V6? */
> > > +		if (!((features & NETIF_F_GEN_CSUM) ||
> > > +		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
> > > +			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
> > >  			if (name)
> > >  				printk(KERN_ERR "%s: Dropping NETIF_F_UFO "
> > > -				       "since no NETIF_F_HW_CSUM feature.\n",
> > > +				       "since no checksum offload features.\n",
> > >  				       name);
> > >  			features &= ~NETIF_F_UFO;
> > >  		}
> > > diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> > > index 956a9f4..d5bc2881 100644
> > > --- a/net/core/ethtool.c
> > > +++ b/net/core/ethtool.c
> > > @@ -1171,7 +1171,9 @@ static int ethtool_set_ufo(struct net_device *dev, char __user *useraddr)
> > >  		return -EFAULT;
> > >  	if (edata.data && !(dev->features & NETIF_F_SG))
> > >  		return -EINVAL;
> > > -	if (edata.data && !(dev->features & NETIF_F_HW_CSUM))
> > > +	if (edata.data && !((dev->features & NETIF_F_GEN_CSUM) ||
> > > +		(dev->features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
> > > +			== (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM)))
> > >  		return -EINVAL;
> > >  	return dev->ethtool_ops->set_ufo(dev, edata.data);
> > >  }
> > I believe UFO is for IPv4 only; IPv6 has an entirely different kind of
> > fragmentation.  So I think the check should be dev->features &
> > NETIF_F_V4_CSUM.
> 
> net/ipv6/ip6_output.c references NETIF_F_UFO without checking
> IPv6 checksumming features.

Got it.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: Bonding, GRO and tcp_reordering
From: Eric Dumazet @ 2010-11-30 16:04 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Simon Horman, netdev
In-Reply-To: <1291131776.21077.27.camel@bwh-desktop>

Le mardi 30 novembre 2010 à 15:42 +0000, Ben Hutchings a écrit :
> On Tue, 2010-11-30 at 22:55 +0900, Simon Horman wrote:

> > The only other parameter that seemed to have significant effect was to
> > increase the mtu.  In the case of MTU=9000, GRO seemed to have a negative
> > impact on throughput, though a significant positive effect on CPU
> > utilisation.
> [...]
> 
> Increasing MTU also increases the interval between packets on a TCP flow
> using maximum segment size so that it is more likely to exceed the
> difference in delay.
> 

GRO really is operational _if_ we receive in same NAPI run several
packets for the same flow.

As soon as we exit NAPI mode, GRO packets are flushed.

Big MTU --> bigger delays between packets, so big chance that GRO cannot
trigger at all, since NAPI runs for one packet only.

One possibility with big MTU is to tweak "ethtool -c eth0" params
rx-usecs: 20
rx-frames: 5
rx-usecs-irq: 0
rx-frames-irq: 5
so that "rx-usecs" is bigger than the delay between two MTU full sized
packets.

Gigabit speed means 1 nano second per bit, and MTU=9000 means 72 us
delay between packets.

So try :

ethtool -C eth0 rx-usecs 100

to get chance that several packets are delivered at once by NIC.

Unfortunately, this also add some latency, so it helps bulk transferts,
and slowdown interactive traffic 



^ permalink raw reply

* Re: [PATCH 3/3] ipip: add module alias for tunl0 tunnel device
From: Stephen Hemminger @ 2010-11-30 16:08 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Changli Gao, David Miller, netdev
In-Reply-To: <1291105269.2725.52.camel@edumazet-laptop>

On Tue, 30 Nov 2010 09:21:09 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le mardi 30 novembre 2010 à 15:55 +0800, Changli Gao a écrit :
> > On Tue, Nov 30, 2010 at 3:19 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > Le lundi 29 novembre 2010 à 11:47 -0800, Stephen Hemminger a écrit :
> > >> pièce jointe document texte brut (ipip-alias.patch)
> > >> If ipip is built as a module the 'ip tunnel add' command would fail because
> > >> the ipip module was not being autoloaded.  Adding an alias for
> > >> the tunl0 device name cause dev_load() to autoload it when needed.
> > >>
> > >> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > >>
> > >> --- a/net/ipv4/ipip.c 2010-11-29 11:40:25.026277890 -0800
> > >> +++ b/net/ipv4/ipip.c 2010-11-29 11:41:05.790681069 -0800
> > >> @@ -913,3 +913,4 @@ static void __exit ipip_fini(void)
> > >>  module_init(ipip_init);
> > >>  module_exit(ipip_fini);
> > >>  MODULE_LICENSE("GPL");
> > >> +MODULE_ALIAS("tunl0");
> > >
> > > I am not sure I understand you...
> > >
> > > Here, I do have ipip built as a module, and I have no problem :
> > >
> > > # lsmod|grep ipip
> > > # ip tunnel add
> > > cannot determine tunnel mode (ipip, gre or sit)
> > > # lsmod|grep ipip
> > > # ip tunnel add mode ipip
> > > # lsmod|grep ipip
> > > ipip                    7692  0
> > > tunnel4                 2949  1 ipip
> > > #
> > >
> > > What am I missing ?
> > >
> > >
> > 
> > localhost linux # lsmod
> > Module                  Size  Used by
> > ipv6                  309359  12
> > localhost linux # ip -V
> > ip utility, iproute2-ss091226
> > localhost linux # ip tunnel add mode gre
> > ioctl: No such device
> > localhost linux # ip tunnel add mode ipip
> > ioctl: No such device
> > localhost linux # modprobe ip_gre
> > localhost linux # modprobe ipip
> > localhost linux # ip tunnel add mode ipip
> > localhost linux # ip tunnel add mode gre
> > localhost linux # lsmod
> > Module                  Size  Used by
> > ipip                    8128  0
> > tunnel4                 2683  1 ipip
> > ip_gre                 15055  0
> > gre                     1967  1 ip_gre
> > ipv6                  309359  13 ip_gre
> > 
> > 
> 
> Hmm. I still dont get it, ipv6 as a module ?
> 
> Works well here.
> 
> # lsmod
> Module                  Size  Used by
> bonding                89194  0 
> ipv6                  232889  29 bonding
> # ip tunnel add mode ipip
> # lsmod
> Module                  Size  Used by
> ipip                    6221  0 
> bonding                89194  0 
> ipv6                  232889  29 bonding
> # ip -V
> ip utility, iproute2-ss100823
> # 

If you run debian there is a list of aliases in /etc/modprobe.d/aliases.conf
that includes the ipip alias.

My patch provides same information from the kernel. In olden times,
the kernel relied more on user space defined aliases, but in the modern
era MODULE_ALIAS() is used to provide that information.


^ permalink raw reply

* Re: [PATCH] net: Fix too optimistic NETIF_F_HW_CSUM features
From: Jon Mason @ 2010-11-30 16:12 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Ramkrishna Vepa, Sivakumar Subramani, Sreenivasa Honnur,
	netdev@vger.kernel.org, Ben Hutchings, Jesse Gross
In-Reply-To: <20101130154123.GA27904@rere.qmqm.pl>

On Tue, Nov 30, 2010 at 07:41:23AM -0800, Michał Mirosław wrote:
> On Tue, Nov 30, 2010 at 04:28:38PM +0100, MichałMirosław wrote:
> > On Tue, Nov 30, 2010 at 03:13:25PM +0000, Ben Hutchings wrote:
> > > On Tue, 2010-11-30 at 15:23 +0100, MichałMirosław wrote:
> > > > NETIF_F_HW_CSUM is superset of NETIF_F_IP_CSUM+NETIF_F_IPV6_CSUM, but
> > > > some drivers miss the difference. Fix this and also fix UFO dependency
> > > > on checksumming offload as it makes the same mistake in assumptions.
> [...]
> > > > diff --git a/drivers/net/vxge/vxge-ethtool.c b/drivers/net/vxge/vxge-ethtool.c
> > > > index bc9bd10..5ece50d 100644
> > > > --- a/drivers/net/vxge/vxge-ethtool.c
> > > > +++ b/drivers/net/vxge/vxge-ethtool.c
> > > > @@ -1177,7 +1177,7 @@ static const struct ethtool_ops vxge_ethtool_ops = {
> > > >  	.get_rx_csum		= vxge_get_rx_csum,
> > > >  	.set_rx_csum		= vxge_set_rx_csum,
> > > >  	.get_tx_csum		= ethtool_op_get_tx_csum,
> > > > -	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
> > > > +	.set_tx_csum		= ethtool_op_set_tx_csum,
> > > >  	.get_sg			= ethtool_op_get_sg,
> > > >  	.set_sg			= ethtool_op_set_sg,
> > > >  	.get_tso		= ethtool_op_get_tso,
> > > > diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> > > > index a21dae1..9f6d379 100644
> > > > --- a/drivers/net/vxge/vxge-main.c
> > > > +++ b/drivers/net/vxge/vxge-main.c
> > > > @@ -3368,7 +3368,7 @@ static int __devinit vxge_device_register(struct __vxge_hw_device *hldev,
> > > >  
> > > >  	ndev->features |= NETIF_F_SG;
> > > >  
> > > > -	ndev->features |= NETIF_F_HW_CSUM;
> > > > +	ndev->features |= NETIF_F_IP_CSUM;
> > > >  	vxge_debug_init(vxge_hw_device_trace_level_get(hldev),
> > > >  		"%s : checksuming enabled", __func__);
> > > >  
> > > Why are you disabling IPv6 checksum offload?  From a quick look at the
> > > driver, I think the hardware does support it.
> > 
> > In vxge_xmit() (at drivers/net/vxge/vxge-main.c:922 in net-next) there
> > is the following code, that suggested otherwise:
> > 
> >         if (skb->ip_summed == CHECKSUM_PARTIAL)
> >                 vxge_hw_fifo_txdl_cksum_set_bits(dtr,
> >                                         VXGE_HW_FIFO_TXD_TX_CKO_IPV4_EN |
> >                                         VXGE_HW_FIFO_TXD_TX_CKO_TCP_EN |
> >                                         VXGE_HW_FIFO_TXD_TX_CKO_UDP_EN);
> > 
> 
> Lets ask vxge driver maintainters on this.
> 
> Does vxge support IPv6 TCP/UDP checksumming offload?

Yes, the underlying hardware can handle checksumming TCP/UDP in an
IPv6 frame.  So the change in the former code above would revert this.
The latter piece of code is for inserting the IPv4 checksum.

Thanks,
Jon

> 
> Best Regards,
> Michał Mirosław

^ permalink raw reply

* [PATCH net-next-2.6] net: dummy: add auto_up module parameter
From: Octavian Purdila @ 2010-11-30 15:57 UTC (permalink / raw)
  To: netdev; +Cc: Vlad Dogaru, Lucian Grijincu, Octavian Purdila

Add auto_up module parameter to automatically bring up the created
devices. This is useful when using the dummy driver for testing net
device register / unregister performance.

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 drivers/net/dummy.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index ff2d29b..ce653b0 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -39,6 +39,7 @@
 #include <linux/u64_stats_sync.h>
 
 static int numdummies = 1;
+static int auto_up;
 
 static int dummy_set_address(struct net_device *dev, void *p)
 {
@@ -132,6 +133,8 @@ static void dummy_setup(struct net_device *dev)
 	/* Fill in device structure with ethernet-generic values. */
 	dev->tx_queue_len = 0;
 	dev->flags |= IFF_NOARP;
+	if (auto_up)
+		dev->flags |= IFF_UP;
 	dev->flags &= ~IFF_MULTICAST;
 	dev->features	|= NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO;
 	dev->features	|= NETIF_F_NO_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
@@ -159,6 +162,9 @@ static struct rtnl_link_ops dummy_link_ops __read_mostly = {
 module_param(numdummies, int, 0);
 MODULE_PARM_DESC(numdummies, "Number of dummy pseudo devices");
 
+module_param(auto_up, int, 0);
+MODULE_PARM_DESC(auto_up, "Automatically bring up the device when created");
+
 static int __init dummy_init_one(void)
 {
 	struct net_device *dev_dummy;
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH] net: Fix too optimistic NETIF_F_HW_CSUM features
From: MichałMirosław @ 2010-11-30 16:18 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, Jesse Gross
In-Reply-To: <1291132318.21077.30.camel@bwh-desktop>

On Tue, Nov 30, 2010 at 03:51:58PM +0000, Ben Hutchings wrote:
> > > Why are you disabling IPv6 checksum offload?  From a quick look at the
> > > driver, I think the hardware does support it.
> > 
> > In vxge_xmit() (at drivers/net/vxge/vxge-main.c:922 in net-next) there
> > is the following code, that suggested otherwise:
> > 
> >         if (skb->ip_summed == CHECKSUM_PARTIAL)
> >                 vxge_hw_fifo_txdl_cksum_set_bits(dtr,
> >                                         VXGE_HW_FIFO_TXD_TX_CKO_IPV4_EN |
> >                                         VXGE_HW_FIFO_TXD_TX_CKO_TCP_EN |
> >                                         VXGE_HW_FIFO_TXD_TX_CKO_UDP_EN);
> 
> I bet IPV4_EN refers to the IPv4 header checksum.  Since IPv6 doesn't
> have a header checksum, there won't be a flag for it here.

That makes sense. I'll fix that.

Thanks,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH 3/3] ipip: add module alias for tunl0 tunnel device
From: Eric Dumazet @ 2010-11-30 16:27 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Changli Gao, David Miller, netdev
In-Reply-To: <20101130080842.685e0b70@nehalam>

Le mardi 30 novembre 2010 à 08:08 -0800, Stephen Hemminger a écrit :

> If you run debian there is a list of aliases in /etc/modprobe.d/aliases.conf
> that includes the ipip alias.
> 
> My patch provides same information from the kernel. In olden times,
> the kernel relied more on user space defined aliases, but in the modern
> era MODULE_ALIAS() is used to provide that information.
> 

Thanks for the clarification.

Running an old Red Hat Enterprise Linux ES release 4 (Nahant Update 8)

# cat /etc/modprobe.conf
alias eth0 bnx2x
alias eth1 bnx2x
alias eth2 tg3
alias eth3 tg3

(No /etc/modprobe.d/ directory)

But I can see the alias in the '.dist' file

# grep ipip /etc/modprobe.conf.dist
alias tunl0 ipip

# mv /etc/modprobe.conf.dist /etc/modprobe.conf.dist.old
# ip tunnel add mode ipip
ioctl: No such device

Thanks !



^ permalink raw reply

* [PATCH 2/5] xfrm: Remove unused ESP padlen field
From: Martin Willi @ 2010-11-30 15:49 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, netdev
In-Reply-To: <1291132155-31277-1-git-send-email-martin@strongswan.org>

The padlen field in IPv4/6 ESP is used to align the ESP padding length
to a value larger than the aead block size. There is however no
option to set this field, hence it is removed.

Signed-off-by: Martin Willi <martin@strongswan.org>
---
 include/net/esp.h |    3 ---
 net/ipv4/esp4.c   |   11 ++---------
 net/ipv6/esp6.c   |   11 ++---------
 3 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/include/net/esp.h b/include/net/esp.h
index d584513..6dfb4d0 100644
--- a/include/net/esp.h
+++ b/include/net/esp.h
@@ -6,9 +6,6 @@
 struct crypto_aead;
 
 struct esp_data {
-	/* 0..255 */
-	int padlen;
-
 	/* Confidentiality & Integrity */
 	struct crypto_aead *aead;
 };
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 14ca1f1..67e4c12 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -132,8 +132,6 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 
 	blksize = ALIGN(crypto_aead_blocksize(aead), 4);
 	clen = ALIGN(clen + 2, blksize);
-	if (esp->padlen)
-		clen = ALIGN(clen, esp->padlen);
 
 	if ((err = skb_cow_data(skb, clen - skb->len + alen, &trailer)) < 0)
 		goto error;
@@ -386,12 +384,11 @@ static u32 esp4_get_mtu(struct xfrm_state *x, int mtu)
 {
 	struct esp_data *esp = x->data;
 	u32 blksize = ALIGN(crypto_aead_blocksize(esp->aead), 4);
-	u32 align = max_t(u32, blksize, esp->padlen);
 	u32 rem;
 
 	mtu -= x->props.header_len + crypto_aead_authsize(esp->aead);
-	rem = mtu & (align - 1);
-	mtu &= ~(align - 1);
+	rem = mtu & (blksize - 1);
+	mtu &= ~(blksize - 1);
 
 	switch (x->props.mode) {
 	case XFRM_MODE_TUNNEL:
@@ -570,8 +567,6 @@ static int esp_init_state(struct xfrm_state *x)
 
 	aead = esp->aead;
 
-	esp->padlen = 0;
-
 	x->props.header_len = sizeof(struct ip_esp_hdr) +
 			      crypto_aead_ivsize(aead);
 	if (x->props.mode == XFRM_MODE_TUNNEL)
@@ -594,8 +589,6 @@ static int esp_init_state(struct xfrm_state *x)
 	}
 
 	align = ALIGN(crypto_aead_blocksize(aead), 4);
-	if (esp->padlen)
-		align = max_t(u32, align, esp->padlen);
 	x->props.trailer_len = align + 1 + crypto_aead_authsize(esp->aead);
 
 error:
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index ee9b93b..e9e6e1c 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -156,8 +156,6 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 
 	blksize = ALIGN(crypto_aead_blocksize(aead), 4);
 	clen = ALIGN(clen + 2, blksize);
-	if (esp->padlen)
-		clen = ALIGN(clen, esp->padlen);
 
 	if ((err = skb_cow_data(skb, clen - skb->len + alen, &trailer)) < 0)
 		goto error;
@@ -337,12 +335,11 @@ static u32 esp6_get_mtu(struct xfrm_state *x, int mtu)
 {
 	struct esp_data *esp = x->data;
 	u32 blksize = ALIGN(crypto_aead_blocksize(esp->aead), 4);
-	u32 align = max_t(u32, blksize, esp->padlen);
 	u32 rem;
 
 	mtu -= x->props.header_len + crypto_aead_authsize(esp->aead);
-	rem = mtu & (align - 1);
-	mtu &= ~(align - 1);
+	rem = mtu & (blksize - 1);
+	mtu &= ~(blksize - 1);
 
 	if (x->props.mode != XFRM_MODE_TUNNEL) {
 		u32 padsize = ((blksize - 1) & 7) + 1;
@@ -516,8 +513,6 @@ static int esp6_init_state(struct xfrm_state *x)
 
 	aead = esp->aead;
 
-	esp->padlen = 0;
-
 	x->props.header_len = sizeof(struct ip_esp_hdr) +
 			      crypto_aead_ivsize(aead);
 	switch (x->props.mode) {
@@ -536,8 +531,6 @@ static int esp6_init_state(struct xfrm_state *x)
 	}
 
 	align = ALIGN(crypto_aead_blocksize(aead), 4);
-	if (esp->padlen)
-		align = max_t(u32, align, esp->padlen);
 	x->props.trailer_len = align + 1 + crypto_aead_authsize(esp->aead);
 
 error:
-- 
1.7.1


^ permalink raw reply related

* [PATCH 4/5] xfrm: Traffic Flow Confidentiality for IPv6 ESP
From: Martin Willi @ 2010-11-30 15:49 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, netdev
In-Reply-To: <1291132155-31277-1-git-send-email-martin@strongswan.org>

If configured on xfrm state, increase the length of all packets to
a given boundary using TFC padding as specified in RFC4303. For
transport mode, or if the XFRM_TFC_ESPV3 is not set, grow the ESP
padding field instead.

Signed-off-by: Martin Willi <martin@strongswan.org>
---
 net/ipv6/esp6.c |   42 +++++++++++++++++++++++++++++++++---------
 1 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index e9e6e1c..9494cb1 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -140,6 +140,9 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 	int blksize;
 	int clen;
 	int alen;
+	int plen;
+	int tfclen;
+	int tfcpadto;
 	int nfrags;
 	u8 *iv;
 	u8 *tail;
@@ -148,16 +151,33 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 	/* skb is pure payload to encrypt */
 	err = -ENOMEM;
 
-	/* Round to block size */
-	clen = skb->len;
-
 	aead = esp->aead;
 	alen = crypto_aead_authsize(aead);
 
 	blksize = ALIGN(crypto_aead_blocksize(aead), 4);
-	clen = ALIGN(clen + 2, blksize);
-
-	if ((err = skb_cow_data(skb, clen - skb->len + alen, &trailer)) < 0)
+	tfclen = 0;
+	tfcpadto = x->tfc.pad;
+
+	if (skb->len >= tfcpadto) {
+		clen = ALIGN(skb->len + 2, blksize);
+	} else if (x->tfc.flags & XFRM_TFC_ESPV3 &&
+		   x->props.mode == XFRM_MODE_TUNNEL) {
+		/* ESPv3 TFC padding, append bytes to payload */
+		tfclen = tfcpadto - skb->len;
+		clen = ALIGN(skb->len + 2 + tfclen, blksize);
+	} else {
+		/* ESPv2 TFC padding. If we exceed the 255 byte maximum, use
+		 * random padding to hide payload length as good as possible. */
+		clen = ALIGN(skb->len + 2 + tfcpadto - skb->len, blksize);
+		if (clen - skb->len - 2 > 255) {
+			clen = ALIGN(skb->len + (u8)random32() + 2, blksize);
+			if (clen - skb->len - 2 > 255)
+				clen -= blksize;
+		}
+	}
+	plen = clen - skb->len - tfclen;
+	err = skb_cow_data(skb, tfclen + plen + alen, &trailer);
+	if (err < 0)
 		goto error;
 	nfrags = err;
 
@@ -172,13 +192,17 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 
 	/* Fill padding... */
 	tail = skb_tail_pointer(trailer);
+	if (tfclen) {
+		memset(tail, 0, tfclen);
+		tail += tfclen;
+	}
 	do {
 		int i;
-		for (i=0; i<clen-skb->len - 2; i++)
+		for (i = 0; i < plen - 2; i++)
 			tail[i] = i + 1;
 	} while (0);
-	tail[clen-skb->len - 2] = (clen - skb->len) - 2;
-	tail[clen - skb->len - 1] = *skb_mac_header(skb);
+	tail[plen - 2] = plen - 2;
+	tail[plen - 1] = *skb_mac_header(skb);
 	pskb_put(skb, trailer, clen - skb->len + alen);
 
 	skb_push(skb, -skb_network_offset(skb));
-- 
1.7.1


^ permalink raw reply related

* [PATCH v2] net: Fix too optimistic NETIF_F_HW_CSUM features
From: MichałMirosław @ 2010-11-30 16:38 UTC (permalink / raw)
  To: netdev
  Cc: Ben Hutchings, Jesse Gross, Jon Mason, Ramkrishna Vepa,
	Sivakumar Subramani, Sreenivasa Honnur
In-Reply-To: <20101130142352.3936.51663.stgit@rechot.qmqm.pl>

NETIF_F_HW_CSUM is a superset of NETIF_F_IP_CSUM+NETIF_F_IPV6_CSUM, but
some drivers miss the difference. Fix this and also fix UFO dependency
on checksumming offload as it makes the same mistake in assumptions.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---

Changes from v1:
- fixed compilation of jme driver
- enable vxge support for IPv6 checksum
---
 drivers/net/vxge/vxge-ethtool.c |    2 +-
 drivers/net/vxge/vxge-main.c    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/benet/be_main.c b/drivers/net/benet/be_main.c
index 102567e..2754280 100644
--- a/drivers/net/benet/be_main.c
+++ b/drivers/net/benet/be_main.c
@@ -2583,10 +2583,12 @@ static void be_netdev_init(struct net_device *netdev)
 	int i;
 
 	netdev->features |= NETIF_F_SG | NETIF_F_HW_VLAN_RX | NETIF_F_TSO |
-		NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_FILTER | NETIF_F_HW_CSUM |
+		NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_FILTER |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 		NETIF_F_GRO | NETIF_F_TSO6;
 
-	netdev->vlan_features |= NETIF_F_SG | NETIF_F_TSO | NETIF_F_HW_CSUM;
+	netdev->vlan_features |= NETIF_F_SG | NETIF_F_TSO |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 
 	if (lancer_chip(adapter))
 		netdev->vlan_features |= NETIF_F_TSO6;
diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index f53edfd..40ce95a 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -8761,7 +8761,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
 	dev->netdev_ops = &bnx2x_netdev_ops;
 	bnx2x_set_ethtool_ops(dev);
 	dev->features |= NETIF_F_SG;
-	dev->features |= NETIF_F_HW_CSUM;
+	dev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	if (bp->flags & USING_DAC_FLAG)
 		dev->features |= NETIF_F_HIGHDMA;
 	dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
@@ -8769,7 +8769,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
 	dev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
 
 	dev->vlan_features |= NETIF_F_SG;
-	dev->vlan_features |= NETIF_F_HW_CSUM;
+	dev->vlan_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	if (bp->flags & USING_DAC_FLAG)
 		dev->vlan_features |= NETIF_F_HIGHDMA;
 	dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index c57d9a4..2411e72 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -2076,12 +2076,11 @@ jme_change_mtu(struct net_device *netdev, int new_mtu)
 	}
 
 	if (new_mtu > 1900) {
-		netdev->features &= ~(NETIF_F_HW_CSUM |
-				NETIF_F_TSO |
-				NETIF_F_TSO6);
+		netdev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+				NETIF_F_TSO | NETIF_F_TSO6);
 	} else {
 		if (test_bit(JME_FLAG_TXCSUM, &jme->flags))
-			netdev->features |= NETIF_F_HW_CSUM;
+			netdev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 		if (test_bit(JME_FLAG_TSO, &jme->flags))
 			netdev->features |= NETIF_F_TSO | NETIF_F_TSO6;
 	}
@@ -2514,10 +2513,12 @@ jme_set_tx_csum(struct net_device *netdev, u32 on)
 	if (on) {
 		set_bit(JME_FLAG_TXCSUM, &jme->flags);
 		if (netdev->mtu <= 1900)
-			netdev->features |= NETIF_F_HW_CSUM;
+			netdev->features |=
+				NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	} else {
 		clear_bit(JME_FLAG_TXCSUM, &jme->flags);
-		netdev->features &= ~NETIF_F_HW_CSUM;
+		netdev->features &=
+				~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
 	}
 
 	return 0;
@@ -2797,7 +2798,8 @@ jme_init_one(struct pci_dev *pdev,
 	netdev->netdev_ops = &jme_netdev_ops;
 	netdev->ethtool_ops		= &jme_ethtool_ops;
 	netdev->watchdog_timeo		= TX_TIMEOUT;
-	netdev->features		=	NETIF_F_HW_CSUM |
+	netdev->features		=	NETIF_F_IP_CSUM |
+						NETIF_F_IPV6_CSUM |
 						NETIF_F_SG |
 						NETIF_F_TSO |
 						NETIF_F_TSO6 |
diff --git a/drivers/net/pch_gbe/pch_gbe_ethtool.c b/drivers/net/pch_gbe/pch_gbe_ethtool.c
index c8cc32c..c8c873b 100644
--- a/drivers/net/pch_gbe/pch_gbe_ethtool.c
+++ b/drivers/net/pch_gbe/pch_gbe_ethtool.c
@@ -469,18 +469,6 @@ static int pch_gbe_set_rx_csum(struct net_device *netdev, u32 data)
 }
 
 /**
- * pch_gbe_get_tx_csum - Report whether transmit checksums are turned on or off
- * @netdev:  Network interface device structure
- * Returns
- *	true(1):  Checksum On
- *	false(0): Checksum Off
- */
-static u32 pch_gbe_get_tx_csum(struct net_device *netdev)
-{
-	return (netdev->features & NETIF_F_HW_CSUM) != 0;
-}
-
-/**
  * pch_gbe_set_tx_csum - Turn transmit checksums on or off
  * @netdev: Network interface device structure
  * @data:   Checksum on[true] or off[false]
@@ -493,11 +481,7 @@ static int pch_gbe_set_tx_csum(struct net_device *netdev, u32 data)
 	struct pch_gbe_adapter *adapter = netdev_priv(netdev);
 
 	adapter->tx_csum = data;
-	if (data)
-		netdev->features |= NETIF_F_HW_CSUM;
-	else
-		netdev->features &= ~NETIF_F_HW_CSUM;
-	return 0;
+	return ethtool_op_set_tx_ipv6_csum(netdev, data);
 }
 
 /**
@@ -572,7 +556,6 @@ static const struct ethtool_ops pch_gbe_ethtool_ops = {
 	.set_pauseparam = pch_gbe_set_pauseparam,
 	.get_rx_csum = pch_gbe_get_rx_csum,
 	.set_rx_csum = pch_gbe_set_rx_csum,
-	.get_tx_csum = pch_gbe_get_tx_csum,
 	.set_tx_csum = pch_gbe_set_tx_csum,
 	.get_strings = pch_gbe_get_strings,
 	.get_ethtool_stats = pch_gbe_get_ethtool_stats,
diff --git a/drivers/net/pch_gbe/pch_gbe_main.c b/drivers/net/pch_gbe/pch_gbe_main.c
index afb7506..58e7903 100644
--- a/drivers/net/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/pch_gbe/pch_gbe_main.c
@@ -2319,7 +2319,7 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	netdev->watchdog_timeo = PCH_GBE_WATCHDOG_PERIOD;
 	netif_napi_add(netdev, &adapter->napi,
 		       pch_gbe_napi_poll, PCH_GBE_RX_WEIGHT);
-	netdev->features = NETIF_F_HW_CSUM | NETIF_F_GRO;
+	netdev->features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_GRO;
 	pch_gbe_set_ethtool_ops(netdev);
 
 	pch_gbe_mac_reset_hw(&adapter->hw);
@@ -2358,9 +2358,9 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	pch_gbe_check_options(adapter);
 
 	if (adapter->tx_csum)
-		netdev->features |= NETIF_F_HW_CSUM;
+		netdev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	else
-		netdev->features &= ~NETIF_F_HW_CSUM;
+		netdev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
 
 	/* initialize the wol settings based on the eeprom settings */
 	adapter->wake_up_evt = PCH_GBE_WL_INIT_SETTING;
diff --git a/drivers/net/sc92031.c b/drivers/net/sc92031.c
index 417adf3..76290a8 100644
--- a/drivers/net/sc92031.c
+++ b/drivers/net/sc92031.c
@@ -1449,7 +1449,8 @@ static int __devinit sc92031_probe(struct pci_dev *pdev,
 	dev->irq = pdev->irq;
 
 	/* faked with skb_copy_and_csum_dev */
-	dev->features = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA;
+	dev->features = NETIF_F_SG | NETIF_F_HIGHDMA |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 
 	dev->netdev_ops		= &sc92031_netdev_ops;
 	dev->watchdog_timeo	= TX_TIMEOUT;
diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index f2695fd..fd719ed 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -197,16 +197,6 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
 	}
 }
 
-static int stmmac_ethtool_set_tx_csum(struct net_device *netdev, u32 data)
-{
-	if (data)
-		netdev->features |= NETIF_F_HW_CSUM;
-	else
-		netdev->features &= ~NETIF_F_HW_CSUM;
-
-	return 0;
-}
-
 static u32 stmmac_ethtool_get_rx_csum(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
@@ -370,7 +360,7 @@ static struct ethtool_ops stmmac_ethtool_ops = {
 	.get_link = ethtool_op_get_link,
 	.get_rx_csum = stmmac_ethtool_get_rx_csum,
 	.get_tx_csum = ethtool_op_get_tx_csum,
-	.set_tx_csum = stmmac_ethtool_set_tx_csum,
+	.set_tx_csum = ethtool_op_set_tx_ipv6_csum,
 	.get_sg = ethtool_op_get_sg,
 	.set_sg = ethtool_op_set_sg,
 	.get_pauseparam = stmmac_get_pauseparam,
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 730a6fd..bfc2d12 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -1494,7 +1494,8 @@ static int stmmac_probe(struct net_device *dev)
 	dev->netdev_ops = &stmmac_netdev_ops;
 	stmmac_set_ethtool_ops(dev);
 
-	dev->features |= (NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_HIGHDMA);
+	dev->features |= NETIF_F_SG | NETIF_F_HIGHDMA |
+		NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	dev->watchdog_timeo = msecs_to_jiffies(watchdog);
 #ifdef STMMAC_VLAN_TAG_USED
 	/* Both mac100 and gmac support receive VLAN tag detection */
@@ -1525,7 +1526,7 @@ static int stmmac_probe(struct net_device *dev)
 
 	DBG(probe, DEBUG, "%s: Scatter/Gather: %s - HW checksums: %s\n",
 	    dev->name, (dev->features & NETIF_F_SG) ? "on" : "off",
-	    (dev->features & NETIF_F_HW_CSUM) ? "on" : "off");
+	    (dev->features & NETIF_F_IP_CSUM) ? "on" : "off");
 
 	spin_lock_init(&priv->lock);
 
diff --git a/drivers/net/vxge/vxge-ethtool.c b/drivers/net/vxge/vxge-ethtool.c
index bc9bd10..1dd3a21 100644
--- a/drivers/net/vxge/vxge-ethtool.c
+++ b/drivers/net/vxge/vxge-ethtool.c
@@ -1177,7 +1177,7 @@ static const struct ethtool_ops vxge_ethtool_ops = {
 	.get_rx_csum		= vxge_get_rx_csum,
 	.set_rx_csum		= vxge_set_rx_csum,
 	.get_tx_csum		= ethtool_op_get_tx_csum,
-	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
+	.set_tx_csum		= ethtool_op_set_tx_ipv6_csum,
 	.get_sg			= ethtool_op_get_sg,
 	.set_sg			= ethtool_op_set_sg,
 	.get_tso		= ethtool_op_get_tso,
diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
index a21dae1..d339f5b 100644
--- a/drivers/net/vxge/vxge-main.c
+++ b/drivers/net/vxge/vxge-main.c
@@ -3368,7 +3368,7 @@ static int __devinit vxge_device_register(struct __vxge_hw_device *hldev,
 
 	ndev->features |= NETIF_F_SG;
 
-	ndev->features |= NETIF_F_HW_CSUM;
+	ndev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
 	vxge_debug_init(vxge_hw_device_trace_level_get(hldev),
 		"%s : checksuming enabled", __func__);
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 3259d2c..622f85a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5041,10 +5041,13 @@ unsigned long netdev_fix_features(unsigned long features, const char *name)
 	}
 
 	if (features & NETIF_F_UFO) {
-		if (!(features & NETIF_F_GEN_CSUM)) {
+		/* maybe split UFO into V4 and V6? */
+		if (!((features & NETIF_F_GEN_CSUM) ||
+		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
+			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
 			if (name)
 				printk(KERN_ERR "%s: Dropping NETIF_F_UFO "
-				       "since no NETIF_F_HW_CSUM feature.\n",
+				       "since no checksum offload features.\n",
 				       name);
 			features &= ~NETIF_F_UFO;
 		}
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 956a9f4..d5bc2881 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1171,7 +1171,9 @@ static int ethtool_set_ufo(struct net_device *dev, char __user *useraddr)
 		return -EFAULT;
 	if (edata.data && !(dev->features & NETIF_F_SG))
 		return -EINVAL;
-	if (edata.data && !(dev->features & NETIF_F_HW_CSUM))
+	if (edata.data && !((dev->features & NETIF_F_GEN_CSUM) ||
+		(dev->features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
+			== (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM)))
 		return -EINVAL;
 	return dev->ethtool_ops->set_ufo(dev, edata.data);
 }


^ permalink raw reply related

* Re: [PATCH net-next-2.6] can: add slcan driver for serial/USB-serial CAN adapters
From: Oliver Hartkopp @ 2010-11-30 16:40 UTC (permalink / raw)
  To: Alan Cox; +Cc: SocketCAN Core Mailing List, Linux Netdev List, David Miller
In-Reply-To: <20101129233718.45dee61d-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>

On 30.11.2010 00:37, Alan Cox wrote:
> On Mon, 29 Nov 2010 21:30:45 +0100
> Oliver Hartkopp <socketcan-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org> wrote:
> 
>> This patch adds support for serial/USB-serial CAN adapters implementing the
>> LAWICEL ASCII protocol for CAN frame transport over serial lines.
>>
>> The driver implements the SLCAN line discipline and is heavily based on the
>> slip.c driver. Therefore the code style remains similar to slip.c to be able
>> to apply changes of the SLIP driver to the SLCAN driver easily.
> 
> It looks almost identical. Would it not be better to either extract the
> common code or put slip and slcann together as one, otherwise changes
> will always get missed as they have to be done twice ?
> 
> How hard would it be to make the encap/decap pointers that can be
> set to cleanly abstract both ?

My first approach was indeed to try to integrate the slcan ldisc into slip.
But due to the higher complexity with dynamic memory for the buffers, the
multiple #ifdef CONFIG_* blocks, the private statistics, the legacy ioctl()
and compat stuff, MTU change, i decided to cut all the unneeded code and make
an easy short driver from that ...

The things i did not really understand i preserved as-is 8-)
Therefore the code looks that similar in some cases.

I wonder whether the code in

- sl_open()
- sl_close()
- slip_write_wakeup()
- sl_xmit()
- sl_free_netdev()
- sl_sync()
- sl_alloc()
- slip_hangup()
- slip_exit()

could be shared e.g. in some separate module where also
drivers/net/hamradio/mkiss.c could participate?!?

This would need to unify the slip/slcan/mkiss structs.

Additionally mkiss.c does not have something like

    static struct net_device **slip_devs;

and creates the netdevices without parsing a fixed number of devices in
mkiss_open(). But as i'm not a tty specialist i better kept my hands off these
internals ...

Regards,
Oliver

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox