Netdev List
 help / color / mirror / Atom feed
* Re: Kernel panic nf_nat_setup_info+0x5b3/0x6e0
From: Eric Dumazet @ 2011-03-31 14:47 UTC (permalink / raw)
  To: "Oleg A. Arkhangelsky"
  Cc: Changli Gao, Patrick McHardy, netfilter-devel, netdev,
	Paul E McKenney
In-Reply-To: <586191301580222@web107.yandex.ru>

Le jeudi 31 mars 2011 à 18:03 +0400, "Oleg A. Arkhangelsky" a écrit :
> 
> 26.03.2011, 16:44, "Changli Gao" <xiaosuo@gmail.com>:
> > On Thu, Mar 3, 2011 at 3:33 PM, Changli Gao <xiaosuo@gmail.com>; wrote:
> >
> >>  Please try the patch attached and test if the problem is solved or not. Thanks.
> >
> > Any feedback? Thanks.
> >
> 
> Seems that patch is fine.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=21512
> 

I wonder if this is not hiding another bug.

Adding an RCU grace period might reduce the probability window.

By the time nf_conntrack_free(ct) is called, no other cpu/thread
could/should use ct, or ct->ext ?

Sure, another thread can find/pass_on ct in a lookup but should not use
it, since its refcount (ct_general.use) should be 0.

Patrick ?



^ permalink raw reply

* Re: [PATCH net-next-2.6 v1] sctp: Add Auto-ASCONF support
From: Michio Honda @ 2011-03-31 14:47 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: netdev, lksctp-developers
In-Reply-To: <4D9422EF.2090007@cn.fujitsu.com>

Hi, 

Thanks for your comments.  
I would like to discuss some of them in-line before resubmission.  

Cheers,
- Michio

On Mar 31, 2011, at 15:45 , Wei Yongjun wrote:

> Hi Michio Honda
> 
> I try to understand what you are doing now, and some comments inline.
> 
> And
> 
> this patch is too big for review, you'd better to split it to a patchset:
> the improve on orig asconf code, the auto-asconf support, the socket
> option etc.
> 
>> @@ -599,6 +597,28 @@ static int sctp_send_asconf_add_ip(struct sock		*sk,
>> 						    SCTP_ADDR_NEW, GFP_ATOMIC);
>> 			addr_buf += af->sockaddr_len;
>> 		}
>> +		list_for_each_entry(trans, &asoc->peer.transport_addr_list,
>> +		    transports) {
>> +			if (asoc->asconf_addr_del_pending != NULL)
>> +				/* This ADDIP ASCONF piggybacks DELIP for the
>> +				 * last address, so need to select src addr
>> +				 * from the out_of_asoc addrs
>> +				 */
>> +				asoc->src_out_of_asoc_ok = 1;
>> +			/* Clear the source and route cache in the path */
>> +			memset(&trans->saddr, 0, sizeof(union sctp_addr));
>> +			dst_release(trans->dst);
>> +			trans->cwnd = min(4*asoc->pathmtu, max_t(__u32,
>> +			    2*asoc->pathmtu, 4380));
>> +			trans->ssthresh = asoc->peer.i.a_rwnd;
>> +			trans->rto = asoc->rto_initial;
>> +			trans->rtt = 0;
>> +			trans->srtt = 0;
>> +			trans->rttvar = 0;
>> +			sctp_transport_route(trans, NULL,
>> +			    sctp_sk(asoc->base.sk));
>> +		}
>> +		retval = sctp_send_asconf(asoc, chunk);
> 
> We need to do this really?
We have to reset path information before transmitting ASCONF from the new source address.  
Because when we obviously change the source address (i.e., deletion of the last local address), we don't know any information of the path (e.g., RTT) between the new source address and the existing destination address.  
> 
>> 
>> @@ -3341,6 +3587,44 @@ static int sctp_setsockopt_del_key(struct sock *sk,
>> 
>> +static int sctp_setsockopt_auto_asconf(struct sock *sk, char __user *optval,
>> +					unsigned int optlen)
>> +{
>> +	int val;
>> +	struct sctp_ep_common *epb;
>> +
>> +	if (optlen < sizeof(int))
>> +		return -EINVAL;
>> +	if (get_user(val, (int __user *)optval))
>> +		return -EFAULT;
>> +	if (!sctp_is_ep_boundall(sk) && val)
>> +		return -EINVAL;
>> +	list_for_each_entry(epb, &sctp_auto_asconf_eplist, auto_asconf_list) {
>> +		if (epb->sk == sk) {
>> +			if (val == 0)
>> +				list_del(&epb->auto_asconf_list);
>> +			return 0;
>> +		}
>> +	}
>> +	if (val)
>> +		list_add_tail(&sctp_sk(sk)->ep->base.auto_asconf_list,
>> +		    &sctp_auto_asconf_eplist);
>> +	return 0;
>> +}
> 
> The sctp_setsockopt_auto_asconf is not correct, if we enable this option
> twice, the list will be broken.
Oops, I fix, thanks!
> 
> We'd better to introcude a auto_asconf flag to ep, and then, we do not
> need to scanf the asconf_list.
> If auto_asconf flag is true, it is exists in the list, otherwise, not.
Agreed, it's more simple.   
> 
> All of the sctp sockets are added to sctp_auto_asconf_eplist.
> I think none of the sockets should exists in this list by default,
> only after we enable AUTO_ASCONF, we should add it to this list.
Sockets are added to auto_asconf_eplist only when the sysctl parameter is on.   
> 
>> 
> 
> Is the sysctl parameter necessary, since auto_asconf is disabled
> by default, and we need to set the option to enabled it.
I believe auto_asconf should be enabled by both of sysctl and setsockopt as well as FreeBSD implementation does.  
Adopting the same methods could be helpful for applications concerning auto_asconf availability.  
I agree to set the sysctl parameter to 0 by default.    

> 
> 
> 



^ permalink raw reply

* Re: Kernel panic nf_nat_setup_info+0x5b3/0x6e0
From: "Oleg A. Arkhangelsky" @ 2011-03-31 14:03 UTC (permalink / raw)
  To: Changli Gao; +Cc: Patrick McHardy, netfilter-devel, netdev, Paul E McKenney
In-Reply-To: <AANLkTi=v_Ckn00N=b2R2pW7jb3BmyyWPSkeCYRxBW0CW@mail.gmail.com>



26.03.2011, 16:44, "Changli Gao" <xiaosuo@gmail.com>:
> On Thu, Mar 3, 2011 at 3:33 PM, Changli Gao <xiaosuo@gmail.com>; wrote:
>
>>  Please try the patch attached and test if the problem is solved or not. Thanks.
>
> Any feedback? Thanks.
>

Seems that patch is fine.

https://bugzilla.kernel.org/show_bug.cgi?id=21512

-- 
wbr, Oleg.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" 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

* RE: [PATCH] net: ethtool support to configure number of RX rings
From: Ben Hutchings @ 2011-03-31 13:05 UTC (permalink / raw)
  To: Amit Salecha
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Ameen Rahman,
	Anirban Chakraborty
In-Reply-To: <99737F4847ED0A48AECC9F4A1974A4B80FD10E8BD6@MNEXMB2.qlogic.org>

On Thu, 2011-03-31 at 00:55 -0500, Amit Salecha wrote:
> > o Ethtool hook to configure number of RX rings.
> > o Implementation is scalable to support configuring TX rings.
> > o There exist ETHTOOL_GRXRINGS command for getting number of RX rings,
> >   but it is tigtly coupled with RX flow hash configuration.
> > o Patches for qlcnic and netxen_nic driver supporting this features
> >   will follow soon.
> >
> > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> > ---
> >  include/linux/ethtool.h |   20 ++++++++++++++++++++
> >  net/core/ethtool.c      |   33 +++++++++++++++++++++++++++++++++
> >  2 files changed, 53 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> > index aac3e2e..de0decd 100644
> > --- a/include/linux/ethtool.h
> > +++ b/include/linux/ethtool.h
> > @@ -226,6 +226,21 @@ struct ethtool_ringparam {
> >       __u32   tx_pending;
> >  };
> >
> > +/* for configuring number of RX ring */
> > +struct ethtool_ring {
> > +     __u32   cmd;    /* ETHTOOL_{G,S}RING */
> > +
> > +     /* Read only attributes.  These indicate the maximum number
> > +      * of RX rings the driver will allow the user to set.
> > +      */
> > +     __u32   rx_max;
> > +
> > +     /* Values changeable by the user.  The valid values are
> > +      * in the range 1 to the "*_max" counterpart above.
> > +      */
> > +     __u32   rx_pending;
> > +};
> > +
> 
> Current implementation can't scale to support TX rings. Because once
> structure is defined, that can't be changed.
> As tools will have dependency on it.
> If configuring tx rings need to be supported, I need to define Tx
> parameters in struct ethtool_ring now.
> Currently qlcnic and netxen_nic driver doesn't have requirement to
> configure number of tx rings.
> Should I define tx parameters in struct ethtool_ring ?

You could take this patch: http://patchwork.ozlabs.org/patch/65202/
but drop the affinity control from it.

Your implementation of set_channels() would then refuse all changes to
the numbers of TX/other/combined channels.

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 v2] net: convert SMSC USB net drivers to hw_features
From: Michał Mirosław @ 2011-03-31 11:39 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning
In-Reply-To: <20110331110135.E66E413A6B@rere.qmqm.pl>

There's a race (not fixed here) in smsc75xx in setting RFE_CTL that's not
properly handled via rfe_ctl_lock. Spinlock is not a good tool here, as
this has to wait for URB completion (or maybe just submission) after issuing
register write request. Otherwise, the rfe_ctl might be changed just after
spin_unlock() and device left programmed with other value.

smsc95xx has increased hard_header_len for the case of TX checksumming.

smsc75xx is fixed to advertise IP+IPV6_CSUM instead of HW_CSUM as it does
not use csum_start/csum_offset.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
[v1 introduced some warnings about unused local variables]

 drivers/net/usb/smsc75xx.c |  124 ++++++++++++++++----------------------------
 drivers/net/usb/smsc95xx.c |   83 +++++++-----------------------
 2 files changed, 63 insertions(+), 144 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 753ee6e..860a20c 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -65,7 +65,6 @@ struct smsc75xx_priv {
 	struct usbnet *dev;
 	u32 rfe_ctl;
 	u32 multicast_hash_table[DP_SEL_VHF_HASH_LEN];
-	bool use_rx_csum;
 	struct mutex dataport_mutex;
 	spinlock_t rfe_ctl_lock;
 	struct work_struct set_multicast;
@@ -548,28 +547,6 @@ static void smsc75xx_status(struct usbnet *dev, struct urb *urb)
 			"unexpected interrupt, intdata=0x%08X", intdata);
 }
 
-/* Enable or disable Rx checksum offload engine */
-static int smsc75xx_set_rx_csum_offload(struct usbnet *dev)
-{
-	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&pdata->rfe_ctl_lock, flags);
-
-	if (pdata->use_rx_csum)
-		pdata->rfe_ctl |= RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM;
-	else
-		pdata->rfe_ctl &= ~(RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM);
-
-	spin_unlock_irqrestore(&pdata->rfe_ctl_lock, flags);
-
-	ret = smsc75xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
-	check_warn_return(ret, "Error writing RFE_CTL");
-
-	return 0;
-}
-
 static int smsc75xx_ethtool_get_eeprom_len(struct net_device *net)
 {
 	return MAX_EEPROM_SIZE;
@@ -599,34 +576,6 @@ static int smsc75xx_ethtool_set_eeprom(struct net_device *netdev,
 	return smsc75xx_write_eeprom(dev, ee->offset, ee->len, data);
 }
 
-static u32 smsc75xx_ethtool_get_rx_csum(struct net_device *netdev)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
-
-	return pdata->use_rx_csum;
-}
-
-static int smsc75xx_ethtool_set_rx_csum(struct net_device *netdev, u32 val)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
-
-	pdata->use_rx_csum = !!val;
-
-	return smsc75xx_set_rx_csum_offload(dev);
-}
-
-static int smsc75xx_ethtool_set_tso(struct net_device *netdev, u32 data)
-{
-	if (data)
-		netdev->features |= NETIF_F_TSO | NETIF_F_TSO6;
-	else
-		netdev->features &= ~(NETIF_F_TSO | NETIF_F_TSO6);
-
-	return 0;
-}
-
 static const struct ethtool_ops smsc75xx_ethtool_ops = {
 	.get_link	= usbnet_get_link,
 	.nway_reset	= usbnet_nway_reset,
@@ -638,12 +587,6 @@ static const struct ethtool_ops smsc75xx_ethtool_ops = {
 	.get_eeprom_len	= smsc75xx_ethtool_get_eeprom_len,
 	.get_eeprom	= smsc75xx_ethtool_get_eeprom,
 	.set_eeprom	= smsc75xx_ethtool_set_eeprom,
-	.get_tx_csum	= ethtool_op_get_tx_csum,
-	.set_tx_csum	= ethtool_op_set_tx_hw_csum,
-	.get_rx_csum	= smsc75xx_ethtool_get_rx_csum,
-	.set_rx_csum	= smsc75xx_ethtool_set_rx_csum,
-	.get_tso	= ethtool_op_get_tso,
-	.set_tso	= smsc75xx_ethtool_set_tso,
 };
 
 static int smsc75xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
@@ -782,6 +725,30 @@ static int smsc75xx_change_mtu(struct net_device *netdev, int new_mtu)
 	return usbnet_change_mtu(netdev, new_mtu);
 }
 
+/* Enable or disable Rx checksum offload engine */
+static int smsc75xx_set_features(struct net_device *netdev, u32 features)
+{
+	struct usbnet *dev = netdev_priv(netdev);
+	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&pdata->rfe_ctl_lock, flags);
+
+	if (features & NETIF_F_RXCSUM)
+		pdata->rfe_ctl |= RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM;
+	else
+		pdata->rfe_ctl &= ~(RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM);
+
+	spin_unlock_irqrestore(&pdata->rfe_ctl_lock, flags);
+	/* it's racing here! */
+
+	ret = smsc75xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
+	check_warn_return(ret, "Error writing RFE_CTL");
+
+	return 0;
+}
+
 static int smsc75xx_reset(struct usbnet *dev)
 {
 	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
@@ -960,11 +927,7 @@ static int smsc75xx_reset(struct usbnet *dev)
 	netif_dbg(dev, ifup, dev->net, "RFE_CTL set to 0x%08x", pdata->rfe_ctl);
 
 	/* Enable or disable checksum offload engines */
-	ethtool_op_set_tx_hw_csum(dev->net, DEFAULT_TX_CSUM_ENABLE);
-	ret = smsc75xx_set_rx_csum_offload(dev);
-	check_warn_return(ret, "Failed to set rx csum offload: %d", ret);
-
-	smsc75xx_ethtool_set_tso(dev->net, DEFAULT_TSO_ENABLE);
+	smsc75xx_set_features(dev->net, dev->net->features);
 
 	smsc75xx_set_multicast(dev->net);
 
@@ -1037,6 +1000,7 @@ static const struct net_device_ops smsc75xx_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl 		= smsc75xx_ioctl,
 	.ndo_set_multicast_list = smsc75xx_set_multicast,
+	.ndo_set_features	= smsc75xx_set_features,
 };
 
 static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
@@ -1065,10 +1029,17 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	INIT_WORK(&pdata->set_multicast, smsc75xx_deferred_multicast_write);
 
-	pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE;
+	if (DEFAULT_TX_CSUM_ENABLE) {
+		dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+		if (DEFAULT_TSO_ENABLE)
+			dev->net->features |= NETIF_F_SG |
+				NETIF_F_TSO | NETIF_F_TSO6;
+	}
+	if (DEFAULT_RX_CSUM_ENABLE)
+		dev->net->features |= NETIF_F_RXCSUM;
 
-	/* We have to advertise SG otherwise TSO cannot be enabled */
-	dev->net->features |= NETIF_F_SG;
+	dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+		NETIF_F_SG | NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_RXCSUM;
 
 	/* Init all registers */
 	ret = smsc75xx_reset(dev);
@@ -1091,10 +1062,11 @@ static void smsc75xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 	}
 }
 
-static void smsc75xx_rx_csum_offload(struct sk_buff *skb, u32 rx_cmd_a,
-				     u32 rx_cmd_b)
+static void smsc75xx_rx_csum_offload(struct usbnet *dev, struct sk_buff *skb,
+				     u32 rx_cmd_a, u32 rx_cmd_b)
 {
-	if (unlikely(rx_cmd_a & RX_CMD_A_LCSM)) {
+	if (!(dev->net->features & NETIF_F_RXCSUM) ||
+	    unlikely(rx_cmd_a & RX_CMD_A_LCSM)) {
 		skb->ip_summed = CHECKSUM_NONE;
 	} else {
 		skb->csum = ntohs((u16)(rx_cmd_b >> RX_CMD_B_CSUM_SHIFT));
@@ -1104,8 +1076,6 @@ static void smsc75xx_rx_csum_offload(struct sk_buff *skb, u32 rx_cmd_a,
 
 static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
-	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
-
 	while (skb->len > 0) {
 		u32 rx_cmd_a, rx_cmd_b, align_count, size;
 		struct sk_buff *ax_skb;
@@ -1145,11 +1115,8 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 			/* last frame in this batch */
 			if (skb->len == size) {
-				if (pdata->use_rx_csum)
-					smsc75xx_rx_csum_offload(skb, rx_cmd_a,
-						rx_cmd_b);
-				else
-					skb->ip_summed = CHECKSUM_NONE;
+				smsc75xx_rx_csum_offload(dev, skb, rx_cmd_a,
+					rx_cmd_b);
 
 				skb_trim(skb, skb->len - 4); /* remove fcs */
 				skb->truesize = size + sizeof(struct sk_buff);
@@ -1167,11 +1134,8 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			ax_skb->data = packet;
 			skb_set_tail_pointer(ax_skb, size);
 
-			if (pdata->use_rx_csum)
-				smsc75xx_rx_csum_offload(ax_skb, rx_cmd_a,
-					rx_cmd_b);
-			else
-				ax_skb->ip_summed = CHECKSUM_NONE;
+			smsc75xx_rx_csum_offload(dev, ax_skb, rx_cmd_a,
+				rx_cmd_b);
 
 			skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
 			ax_skb->truesize = size + sizeof(struct sk_buff);
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 727874d..708f208 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -52,8 +52,6 @@ struct smsc95xx_priv {
 	u32 hash_hi;
 	u32 hash_lo;
 	spinlock_t mac_cr_lock;
-	bool use_tx_csum;
-	bool use_rx_csum;
 };
 
 struct usb_context {
@@ -517,22 +515,24 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
 }
 
 /* Enable or disable Tx & Rx checksum offload engines */
-static int smsc95xx_set_csums(struct usbnet *dev)
+static int smsc95xx_set_features(struct net_device *netdev, u32 features)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct usbnet *dev = netdev_priv(netdev);
 	u32 read_buf;
-	int ret = smsc95xx_read_reg(dev, COE_CR, &read_buf);
+	int ret;
+
+	ret = smsc95xx_read_reg(dev, COE_CR, &read_buf);
 	if (ret < 0) {
 		netdev_warn(dev->net, "Failed to read COE_CR: %d\n", ret);
 		return ret;
 	}
 
-	if (pdata->use_tx_csum)
+	if (features & NETIF_F_HW_CSUM)
 		read_buf |= Tx_COE_EN_;
 	else
 		read_buf &= ~Tx_COE_EN_;
 
-	if (pdata->use_rx_csum)
+	if (features & NETIF_F_RXCSUM)
 		read_buf |= Rx_COE_EN_;
 	else
 		read_buf &= ~Rx_COE_EN_;
@@ -576,43 +576,6 @@ static int smsc95xx_ethtool_set_eeprom(struct net_device *netdev,
 	return smsc95xx_write_eeprom(dev, ee->offset, ee->len, data);
 }
 
-static u32 smsc95xx_ethtool_get_rx_csum(struct net_device *netdev)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	return pdata->use_rx_csum;
-}
-
-static int smsc95xx_ethtool_set_rx_csum(struct net_device *netdev, u32 val)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	pdata->use_rx_csum = !!val;
-
-	return smsc95xx_set_csums(dev);
-}
-
-static u32 smsc95xx_ethtool_get_tx_csum(struct net_device *netdev)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	return pdata->use_tx_csum;
-}
-
-static int smsc95xx_ethtool_set_tx_csum(struct net_device *netdev, u32 val)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	pdata->use_tx_csum = !!val;
-
-	ethtool_op_set_tx_hw_csum(netdev, pdata->use_tx_csum);
-	return smsc95xx_set_csums(dev);
-}
-
 static const struct ethtool_ops smsc95xx_ethtool_ops = {
 	.get_link	= usbnet_get_link,
 	.nway_reset	= usbnet_nway_reset,
@@ -624,10 +587,6 @@ static const struct ethtool_ops smsc95xx_ethtool_ops = {
 	.get_eeprom_len	= smsc95xx_ethtool_get_eeprom_len,
 	.get_eeprom	= smsc95xx_ethtool_get_eeprom,
 	.set_eeprom	= smsc95xx_ethtool_set_eeprom,
-	.get_tx_csum	= smsc95xx_ethtool_get_tx_csum,
-	.set_tx_csum	= smsc95xx_ethtool_set_tx_csum,
-	.get_rx_csum	= smsc95xx_ethtool_get_rx_csum,
-	.set_rx_csum	= smsc95xx_ethtool_set_rx_csum,
 };
 
 static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
@@ -755,7 +714,6 @@ static int smsc95xx_phy_initialize(struct usbnet *dev)
 static int smsc95xx_reset(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-	struct net_device *netdev = dev->net;
 	u32 read_buf, write_buf, burst_cap;
 	int ret = 0, timeout;
 
@@ -975,12 +933,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 	}
 
 	/* Enable or disable checksum offload engines */
-	ethtool_op_set_tx_hw_csum(netdev, pdata->use_tx_csum);
-	ret = smsc95xx_set_csums(dev);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to set csum offload: %d\n", ret);
-		return ret;
-	}
+	smsc95xx_set_features(dev->net, dev->net->features);
 
 	smsc95xx_set_multicast(dev->net);
 
@@ -1019,6 +972,7 @@ static const struct net_device_ops smsc95xx_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl 		= smsc95xx_ioctl,
 	.ndo_set_multicast_list = smsc95xx_set_multicast,
+	.ndo_set_features	= smsc95xx_set_features,
 };
 
 static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
@@ -1045,8 +999,12 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	spin_lock_init(&pdata->mac_cr_lock);
 
-	pdata->use_tx_csum = DEFAULT_TX_CSUM_ENABLE;
-	pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE;
+	if (DEFAULT_TX_CSUM_ENABLE)
+		dev->net->features |= NETIF_F_HW_CSUM;
+	if (DEFAULT_RX_CSUM_ENABLE)
+		dev->net->features |= NETIF_F_RXCSUM;
+
+	dev->net->hw_features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
 
 	smsc95xx_init_mac_address(dev);
 
@@ -1056,7 +1014,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->netdev_ops = &smsc95xx_netdev_ops;
 	dev->net->ethtool_ops = &smsc95xx_ethtool_ops;
 	dev->net->flags |= IFF_MULTICAST;
-	dev->net->hard_header_len += SMSC95XX_TX_OVERHEAD;
+	dev->net->hard_header_len += SMSC95XX_TX_OVERHEAD_CSUM;
 	return 0;
 }
 
@@ -1080,8 +1038,6 @@ static void smsc95xx_rx_csum_offload(struct sk_buff *skb)
 
 static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
 	while (skb->len > 0) {
 		u32 header, align_count;
 		struct sk_buff *ax_skb;
@@ -1123,7 +1079,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 			/* last frame in this batch */
 			if (skb->len == size) {
-				if (pdata->use_rx_csum)
+				if (dev->net->features & NETIF_F_RXCSUM)
 					smsc95xx_rx_csum_offload(skb);
 				skb_trim(skb, skb->len - 4); /* remove fcs */
 				skb->truesize = size + sizeof(struct sk_buff);
@@ -1141,7 +1097,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			ax_skb->data = packet;
 			skb_set_tail_pointer(ax_skb, size);
 
-			if (pdata->use_rx_csum)
+			if (dev->net->features & NETIF_F_RXCSUM)
 				smsc95xx_rx_csum_offload(ax_skb);
 			skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
 			ax_skb->truesize = size + sizeof(struct sk_buff);
@@ -1174,8 +1130,7 @@ static u32 smsc95xx_calc_csum_preamble(struct sk_buff *skb)
 static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 					 struct sk_buff *skb, gfp_t flags)
 {
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-	bool csum = pdata->use_tx_csum && (skb->ip_summed == CHECKSUM_PARTIAL);
+	bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
 	int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
 	u32 tx_cmd_a, tx_cmd_b;
 
-- 
1.7.2.5


^ permalink raw reply related

* Re: [PATCH RESEND] net: convert xen-netfront to hw_features
From: Michał Mirosław @ 2011-03-31 11:37 UTC (permalink / raw)
  To: Ian Campbell
  Cc: netdev@vger.kernel.org, Jeremy Fitzhardinge,
	Konrad Rzeszutek Wilk, xen-devel@lists.xensource.com,
	virtualization@lists.linux-foundation.org
In-Reply-To: <1301570010.27123.172.camel@zakaz.uk.xensource.com>

On Thu, Mar 31, 2011 at 12:13:30PM +0100, Ian Campbell wrote:
> On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote:
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> Thanks, I've got a pending TODO item to test this and propagate similar
> changes to netback. I hope to get to it soon...
> 
> Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for
> a long time now...

This probably affects only people who are using suspending VM's to disk or
[live] migration. The bug is hard to notice if you're not looking for it.
I only noticed it after trying to change the code.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH RESEND] net: convert xen-netfront to hw_features
From: Ian Campbell @ 2011-03-31 11:13 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: netdev@vger.kernel.org, Jeremy Fitzhardinge,
	Konrad Rzeszutek Wilk, xen-devel@lists.xensource.com,
	virtualization@lists.linux-foundation.org
In-Reply-To: <20110331110136.03A1A13A6A@rere.qmqm.pl>

On Thu, 2011-03-31 at 12:01 +0100, Michał Mirosław wrote:
> Not tested in any way. The original code for offload setting seems broken
> as it resets the features on every netback reconnect.

Thanks, I've got a pending TODO item to test this and propagate similar
changes to netback. I hope to get to it soon...

Is this urgent (for 2.6.39) IYHO? I think it's been broken this way for
a long time now...

Ian.

> 
> This will set GSO_ROBUST at device creation time (earlier than connect time).
> 
> RX checksum offload is forced on - so advertise as it is.
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> [I don't know Xen code enough to say this is correct. There is Xen netback
> driver coming in, that has similar changes to be made. Please match
> them up if you can.]
> 
>  drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
>  1 files changed, 23 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 5c8d9c3..2a71c9f 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
>  	.ndo_change_mtu	     = xennet_change_mtu,
>  	.ndo_set_mac_address = eth_mac_addr,
>  	.ndo_validate_addr   = eth_validate_addr,
> +	.ndo_fix_features    = xennet_fix_features,
> +	.ndo_set_features    = xennet_set_features,
>  };
>  
>  static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
> @@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
>  	netdev->netdev_ops	= &xennet_netdev_ops;
>  
>  	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
> -	netdev->features        = NETIF_F_IP_CSUM;
> +	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> +				  NETIF_F_GSO_ROBUST;
> +	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
>  
>  	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
>  	SET_NETDEV_DEV(netdev, &dev->dev);
> @@ -1510,52 +1514,40 @@ again:
>  	return err;
>  }
>  
> -static int xennet_set_sg(struct net_device *dev, u32 data)
> +static u32 xennet_fix_features(struct net_device *dev, u32 features)
>  {
> -	if (data) {
> -		struct netfront_info *np = netdev_priv(dev);
> -		int val;
> +	struct netfront_info *np = netdev_priv(dev);
> +	int val;
>  
> +	if (features & NETIF_F_SG) {
>  		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
>  				 "%d", &val) < 0)
>  			val = 0;
> +
>  		if (!val)
> -			return -ENOSYS;
> -	} else if (dev->mtu > ETH_DATA_LEN)
> -		dev->mtu = ETH_DATA_LEN;
> -
> -	return ethtool_op_set_sg(dev, data);
> -}
> -
> -static int xennet_set_tso(struct net_device *dev, u32 data)
> -{
> -	if (data) {
> -		struct netfront_info *np = netdev_priv(dev);
> -		int val;
> +			features &= ~NETIF_F_SG;
> +	}
>  
> +	if (features & NETIF_F_TSO) {
>  		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
>  				 "feature-gso-tcpv4", "%d", &val) < 0)
>  			val = 0;
> +
>  		if (!val)
> -			return -ENOSYS;
> +			features &= ~NETIF_F_TSO;
>  	}
>  
> -	return ethtool_op_set_tso(dev, data);
> +	return features;
>  }
>  
> -static void xennet_set_features(struct net_device *dev)
> +static int xennet_set_features(struct net_device *dev, u32 features)
>  {
> -	/* Turn off all GSO bits except ROBUST. */
> -	dev->features &= ~NETIF_F_GSO_MASK;
> -	dev->features |= NETIF_F_GSO_ROBUST;
> -	xennet_set_sg(dev, 0);
> +	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
> +		netdev_info(dev, "Reducing MTU because no SG offload");
> +		dev->mtu = ETH_DATA_LEN;
> +	}
>  
> -	/* We need checksum offload to enable scatter/gather and TSO. */
> -	if (!(dev->features & NETIF_F_IP_CSUM))
> -		return;
> -
> -	if (!xennet_set_sg(dev, 1))
> -		xennet_set_tso(dev, 1);
> +	return 0;
>  }
>  
>  static int xennet_connect(struct net_device *dev)
> @@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
>  	if (err)
>  		return err;
>  
> -	xennet_set_features(dev);
> +	netdev_update_features(dev);
>  
>  	spin_lock_bh(&np->rx_lock);
>  	spin_lock_irq(&np->tx_lock);
> @@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
>  
>  static const struct ethtool_ops xennet_ethtool_ops =
>  {
> -	.set_tx_csum = ethtool_op_set_tx_csum,
> -	.set_sg = xennet_set_sg,
> -	.set_tso = xennet_set_tso,
>  	.get_link = ethtool_op_get_link,
>  
>  	.get_sset_count = xennet_get_sset_count,



^ permalink raw reply

* [PATCH] net: convert SMSC USB net drivers to hw_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning

There's a race (not fixed here) in smsc75xx in setting RFE_CTL that's not
properly handled via rfe_ctl_lock. Spinlock is not a good tool here, as
this has to wait for URB completion (or maybe just submission) after issuing
register write request. Otherwise, the rfe_ctl might be changed just after
spin_unlock() and device left programmed with other value.

smsc95xx has increased hard_header_len for the case of TX checksumming.

smsc75xx is fixed to advertise IP+IPV6_CSUM instead of HW_CSUM as it does
not use csum_start/csum_offset.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
[Not so trivial conversion to hw_features. Please update this if needed
as I don't have the hardware to test.]

 drivers/net/usb/smsc75xx.c |  122 ++++++++++++++++----------------------------
 drivers/net/usb/smsc95xx.c |   74 ++++++---------------------
 2 files changed, 60 insertions(+), 136 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 753ee6e..33c7756 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -65,7 +65,6 @@ struct smsc75xx_priv {
 	struct usbnet *dev;
 	u32 rfe_ctl;
 	u32 multicast_hash_table[DP_SEL_VHF_HASH_LEN];
-	bool use_rx_csum;
 	struct mutex dataport_mutex;
 	spinlock_t rfe_ctl_lock;
 	struct work_struct set_multicast;
@@ -548,28 +547,6 @@ static void smsc75xx_status(struct usbnet *dev, struct urb *urb)
 			"unexpected interrupt, intdata=0x%08X", intdata);
 }
 
-/* Enable or disable Rx checksum offload engine */
-static int smsc75xx_set_rx_csum_offload(struct usbnet *dev)
-{
-	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&pdata->rfe_ctl_lock, flags);
-
-	if (pdata->use_rx_csum)
-		pdata->rfe_ctl |= RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM;
-	else
-		pdata->rfe_ctl &= ~(RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM);
-
-	spin_unlock_irqrestore(&pdata->rfe_ctl_lock, flags);
-
-	ret = smsc75xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
-	check_warn_return(ret, "Error writing RFE_CTL");
-
-	return 0;
-}
-
 static int smsc75xx_ethtool_get_eeprom_len(struct net_device *net)
 {
 	return MAX_EEPROM_SIZE;
@@ -599,34 +576,6 @@ static int smsc75xx_ethtool_set_eeprom(struct net_device *netdev,
 	return smsc75xx_write_eeprom(dev, ee->offset, ee->len, data);
 }
 
-static u32 smsc75xx_ethtool_get_rx_csum(struct net_device *netdev)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
-
-	return pdata->use_rx_csum;
-}
-
-static int smsc75xx_ethtool_set_rx_csum(struct net_device *netdev, u32 val)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
-
-	pdata->use_rx_csum = !!val;
-
-	return smsc75xx_set_rx_csum_offload(dev);
-}
-
-static int smsc75xx_ethtool_set_tso(struct net_device *netdev, u32 data)
-{
-	if (data)
-		netdev->features |= NETIF_F_TSO | NETIF_F_TSO6;
-	else
-		netdev->features &= ~(NETIF_F_TSO | NETIF_F_TSO6);
-
-	return 0;
-}
-
 static const struct ethtool_ops smsc75xx_ethtool_ops = {
 	.get_link	= usbnet_get_link,
 	.nway_reset	= usbnet_nway_reset,
@@ -638,12 +587,6 @@ static const struct ethtool_ops smsc75xx_ethtool_ops = {
 	.get_eeprom_len	= smsc75xx_ethtool_get_eeprom_len,
 	.get_eeprom	= smsc75xx_ethtool_get_eeprom,
 	.set_eeprom	= smsc75xx_ethtool_set_eeprom,
-	.get_tx_csum	= ethtool_op_get_tx_csum,
-	.set_tx_csum	= ethtool_op_set_tx_hw_csum,
-	.get_rx_csum	= smsc75xx_ethtool_get_rx_csum,
-	.set_rx_csum	= smsc75xx_ethtool_set_rx_csum,
-	.get_tso	= ethtool_op_get_tso,
-	.set_tso	= smsc75xx_ethtool_set_tso,
 };
 
 static int smsc75xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
@@ -782,6 +725,30 @@ static int smsc75xx_change_mtu(struct net_device *netdev, int new_mtu)
 	return usbnet_change_mtu(netdev, new_mtu);
 }
 
+/* Enable or disable Rx checksum offload engine */
+static int smsc75xx_set_features(struct net_device *netdev, u32 features)
+{
+	struct usbnet *dev = netdev_priv(netdev);
+	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&pdata->rfe_ctl_lock, flags);
+
+	if (features & NETIF_F_RXCSUM)
+		pdata->rfe_ctl |= RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM;
+	else
+		pdata->rfe_ctl &= ~(RFE_CTL_TCPUDP_CKM | RFE_CTL_IP_CKM);
+
+	spin_unlock_irqrestore(&pdata->rfe_ctl_lock, flags);
+	/* it's racing here! */
+
+	ret = smsc75xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
+	check_warn_return(ret, "Error writing RFE_CTL");
+
+	return 0;
+}
+
 static int smsc75xx_reset(struct usbnet *dev)
 {
 	struct smsc75xx_priv *pdata = (struct smsc75xx_priv *)(dev->data[0]);
@@ -960,11 +927,7 @@ static int smsc75xx_reset(struct usbnet *dev)
 	netif_dbg(dev, ifup, dev->net, "RFE_CTL set to 0x%08x", pdata->rfe_ctl);
 
 	/* Enable or disable checksum offload engines */
-	ethtool_op_set_tx_hw_csum(dev->net, DEFAULT_TX_CSUM_ENABLE);
-	ret = smsc75xx_set_rx_csum_offload(dev);
-	check_warn_return(ret, "Failed to set rx csum offload: %d", ret);
-
-	smsc75xx_ethtool_set_tso(dev->net, DEFAULT_TSO_ENABLE);
+	smsc75xx_set_features(dev->net, dev->net->features);
 
 	smsc75xx_set_multicast(dev->net);
 
@@ -1037,6 +1000,7 @@ static const struct net_device_ops smsc75xx_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl 		= smsc75xx_ioctl,
 	.ndo_set_multicast_list = smsc75xx_set_multicast,
+	.ndo_set_features	= smsc75xx_set_features,
 };
 
 static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
@@ -1065,10 +1029,17 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	INIT_WORK(&pdata->set_multicast, smsc75xx_deferred_multicast_write);
 
-	pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE;
+	if (DEFAULT_TX_CSUM_ENABLE) {
+		dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+		if (DEFAULT_TSO_ENABLE)
+			dev->net->features |= NETIF_F_SG |
+				NETIF_F_TSO | NETIF_F_TSO6;
+	}
+	if (DEFAULT_RX_CSUM_ENABLE)
+		dev->net->features |= NETIF_F_RXCSUM;
 
-	/* We have to advertise SG otherwise TSO cannot be enabled */
-	dev->net->features |= NETIF_F_SG;
+	dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+		NETIF_F_SG | NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_RXCSUM;
 
 	/* Init all registers */
 	ret = smsc75xx_reset(dev);
@@ -1091,10 +1062,11 @@ static void smsc75xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 	}
 }
 
-static void smsc75xx_rx_csum_offload(struct sk_buff *skb, u32 rx_cmd_a,
-				     u32 rx_cmd_b)
+static void smsc75xx_rx_csum_offload(struct usbnet *dev, struct sk_buff *skb,
+				     u32 rx_cmd_a, u32 rx_cmd_b)
 {
-	if (unlikely(rx_cmd_a & RX_CMD_A_LCSM)) {
+	if (!(dev->net->features & NETIF_F_RXCSUM) ||
+	    unlikely(rx_cmd_a & RX_CMD_A_LCSM)) {
 		skb->ip_summed = CHECKSUM_NONE;
 	} else {
 		skb->csum = ntohs((u16)(rx_cmd_b >> RX_CMD_B_CSUM_SHIFT));
@@ -1145,11 +1117,8 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 			/* last frame in this batch */
 			if (skb->len == size) {
-				if (pdata->use_rx_csum)
-					smsc75xx_rx_csum_offload(skb, rx_cmd_a,
-						rx_cmd_b);
-				else
-					skb->ip_summed = CHECKSUM_NONE;
+				smsc75xx_rx_csum_offload(dev, skb, rx_cmd_a,
+					rx_cmd_b);
 
 				skb_trim(skb, skb->len - 4); /* remove fcs */
 				skb->truesize = size + sizeof(struct sk_buff);
@@ -1167,11 +1136,8 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			ax_skb->data = packet;
 			skb_set_tail_pointer(ax_skb, size);
 
-			if (pdata->use_rx_csum)
-				smsc75xx_rx_csum_offload(ax_skb, rx_cmd_a,
-					rx_cmd_b);
-			else
-				ax_skb->ip_summed = CHECKSUM_NONE;
+			smsc75xx_rx_csum_offload(dev, ax_skb, rx_cmd_a,
+				rx_cmd_b);
 
 			skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
 			ax_skb->truesize = size + sizeof(struct sk_buff);
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 727874d..b4f6cc3 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -52,8 +52,6 @@ struct smsc95xx_priv {
 	u32 hash_hi;
 	u32 hash_lo;
 	spinlock_t mac_cr_lock;
-	bool use_tx_csum;
-	bool use_rx_csum;
 };
 
 struct usb_context {
@@ -517,8 +515,9 @@ static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
 }
 
 /* Enable or disable Tx & Rx checksum offload engines */
-static int smsc95xx_set_csums(struct usbnet *dev)
+static int smsc95xx_set_features(struct net_device *netdev, u32 features)
 {
+	struct usbnet *dev = netdev_priv(netdev);
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
 	u32 read_buf;
 	int ret = smsc95xx_read_reg(dev, COE_CR, &read_buf);
@@ -527,12 +526,12 @@ static int smsc95xx_set_csums(struct usbnet *dev)
 		return ret;
 	}
 
-	if (pdata->use_tx_csum)
+	if (features & NETIF_F_HW_CSUM)
 		read_buf |= Tx_COE_EN_;
 	else
 		read_buf &= ~Tx_COE_EN_;
 
-	if (pdata->use_rx_csum)
+	if (features & NETIF_F_RXCSUM)
 		read_buf |= Rx_COE_EN_;
 	else
 		read_buf &= ~Rx_COE_EN_;
@@ -576,43 +575,6 @@ static int smsc95xx_ethtool_set_eeprom(struct net_device *netdev,
 	return smsc95xx_write_eeprom(dev, ee->offset, ee->len, data);
 }
 
-static u32 smsc95xx_ethtool_get_rx_csum(struct net_device *netdev)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	return pdata->use_rx_csum;
-}
-
-static int smsc95xx_ethtool_set_rx_csum(struct net_device *netdev, u32 val)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	pdata->use_rx_csum = !!val;
-
-	return smsc95xx_set_csums(dev);
-}
-
-static u32 smsc95xx_ethtool_get_tx_csum(struct net_device *netdev)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	return pdata->use_tx_csum;
-}
-
-static int smsc95xx_ethtool_set_tx_csum(struct net_device *netdev, u32 val)
-{
-	struct usbnet *dev = netdev_priv(netdev);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-
-	pdata->use_tx_csum = !!val;
-
-	ethtool_op_set_tx_hw_csum(netdev, pdata->use_tx_csum);
-	return smsc95xx_set_csums(dev);
-}
-
 static const struct ethtool_ops smsc95xx_ethtool_ops = {
 	.get_link	= usbnet_get_link,
 	.nway_reset	= usbnet_nway_reset,
@@ -624,10 +586,6 @@ static const struct ethtool_ops smsc95xx_ethtool_ops = {
 	.get_eeprom_len	= smsc95xx_ethtool_get_eeprom_len,
 	.get_eeprom	= smsc95xx_ethtool_get_eeprom,
 	.set_eeprom	= smsc95xx_ethtool_set_eeprom,
-	.get_tx_csum	= smsc95xx_ethtool_get_tx_csum,
-	.set_tx_csum	= smsc95xx_ethtool_set_tx_csum,
-	.get_rx_csum	= smsc95xx_ethtool_get_rx_csum,
-	.set_rx_csum	= smsc95xx_ethtool_set_rx_csum,
 };
 
 static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
@@ -975,12 +933,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 	}
 
 	/* Enable or disable checksum offload engines */
-	ethtool_op_set_tx_hw_csum(netdev, pdata->use_tx_csum);
-	ret = smsc95xx_set_csums(dev);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Failed to set csum offload: %d\n", ret);
-		return ret;
-	}
+	smsc95xx_set_features(dev->net, dev->net->features);
 
 	smsc95xx_set_multicast(dev->net);
 
@@ -1019,6 +972,7 @@ static const struct net_device_ops smsc95xx_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_do_ioctl 		= smsc95xx_ioctl,
 	.ndo_set_multicast_list = smsc95xx_set_multicast,
+	.ndo_set_features	= smsc95xx_set_features,
 };
 
 static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
@@ -1045,8 +999,12 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	spin_lock_init(&pdata->mac_cr_lock);
 
-	pdata->use_tx_csum = DEFAULT_TX_CSUM_ENABLE;
-	pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE;
+	if (DEFAULT_TX_CSUM_ENABLE)
+		dev->net->features |= NETIF_F_HW_CSUM;
+	if (DEFAULT_RX_CSUM_ENABLE)
+		dev->net->features |= NETIF_F_RXCSUM;
+
+	dev->net->hw_features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
 
 	smsc95xx_init_mac_address(dev);
 
@@ -1056,7 +1014,7 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->netdev_ops = &smsc95xx_netdev_ops;
 	dev->net->ethtool_ops = &smsc95xx_ethtool_ops;
 	dev->net->flags |= IFF_MULTICAST;
-	dev->net->hard_header_len += SMSC95XX_TX_OVERHEAD;
+	dev->net->hard_header_len += SMSC95XX_TX_OVERHEAD_CSUM;
 	return 0;
 }
 
@@ -1123,7 +1081,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 			/* last frame in this batch */
 			if (skb->len == size) {
-				if (pdata->use_rx_csum)
+				if (dev->net->features & NETIF_F_RXCSUM)
 					smsc95xx_rx_csum_offload(skb);
 				skb_trim(skb, skb->len - 4); /* remove fcs */
 				skb->truesize = size + sizeof(struct sk_buff);
@@ -1141,7 +1099,7 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 			ax_skb->data = packet;
 			skb_set_tail_pointer(ax_skb, size);
 
-			if (pdata->use_rx_csum)
+			if (dev->net->features & NETIF_F_RXCSUM)
 				smsc95xx_rx_csum_offload(ax_skb);
 			skb_trim(ax_skb, ax_skb->len - 4); /* remove fcs */
 			ax_skb->truesize = size + sizeof(struct sk_buff);
@@ -1175,7 +1133,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 					 struct sk_buff *skb, gfp_t flags)
 {
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
-	bool csum = pdata->use_tx_csum && (skb->ip_summed == CHECKSUM_PARTIAL);
+	bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
 	int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
 	u32 tx_cmd_a, tx_cmd_b;
 
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH RESEND] net: convert xen-netfront to hw_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev
  Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk, Ian Campbell,
	xen-devel, virtualization

Not tested in any way. The original code for offload setting seems broken
as it resets the features on every netback reconnect.

This will set GSO_ROBUST at device creation time (earlier than connect time).

RX checksum offload is forced on - so advertise as it is.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
[I don't know Xen code enough to say this is correct. There is Xen netback
driver coming in, that has similar changes to be made. Please match
them up if you can.]

 drivers/net/xen-netfront.c |   57 +++++++++++++++++--------------------------
 1 files changed, 23 insertions(+), 34 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 5c8d9c3..2a71c9f 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1148,6 +1148,8 @@ static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_change_mtu	     = xennet_change_mtu,
 	.ndo_set_mac_address = eth_mac_addr,
 	.ndo_validate_addr   = eth_validate_addr,
+	.ndo_fix_features    = xennet_fix_features,
+	.ndo_set_features    = xennet_set_features,
 };
 
 static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev)
@@ -1209,7 +1211,9 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 	netdev->netdev_ops	= &xennet_netdev_ops;
 
 	netif_napi_add(netdev, &np->napi, xennet_poll, 64);
-	netdev->features        = NETIF_F_IP_CSUM;
+	netdev->features        = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
+				  NETIF_F_GSO_ROBUST;
+	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
@@ -1510,52 +1514,40 @@ again:
 	return err;
 }
 
-static int xennet_set_sg(struct net_device *dev, u32 data)
+static u32 xennet_fix_features(struct net_device *dev, u32 features)
 {
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+	struct netfront_info *np = netdev_priv(dev);
+	int val;
 
+	if (features & NETIF_F_SG) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend, "feature-sg",
 				 "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
-	} else if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
-
-	return ethtool_op_set_sg(dev, data);
-}
-
-static int xennet_set_tso(struct net_device *dev, u32 data)
-{
-	if (data) {
-		struct netfront_info *np = netdev_priv(dev);
-		int val;
+			features &= ~NETIF_F_SG;
+	}
 
+	if (features & NETIF_F_TSO) {
 		if (xenbus_scanf(XBT_NIL, np->xbdev->otherend,
 				 "feature-gso-tcpv4", "%d", &val) < 0)
 			val = 0;
+
 		if (!val)
-			return -ENOSYS;
+			features &= ~NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	return features;
 }
 
-static void xennet_set_features(struct net_device *dev)
+static int xennet_set_features(struct net_device *dev, u32 features)
 {
-	/* Turn off all GSO bits except ROBUST. */
-	dev->features &= ~NETIF_F_GSO_MASK;
-	dev->features |= NETIF_F_GSO_ROBUST;
-	xennet_set_sg(dev, 0);
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN) {
+		netdev_info(dev, "Reducing MTU because no SG offload");
+		dev->mtu = ETH_DATA_LEN;
+	}
 
-	/* We need checksum offload to enable scatter/gather and TSO. */
-	if (!(dev->features & NETIF_F_IP_CSUM))
-		return;
-
-	if (!xennet_set_sg(dev, 1))
-		xennet_set_tso(dev, 1);
+	return 0;
 }
 
 static int xennet_connect(struct net_device *dev)
@@ -1582,7 +1574,7 @@ static int xennet_connect(struct net_device *dev)
 	if (err)
 		return err;
 
-	xennet_set_features(dev);
+	netdev_update_features(dev);
 
 	spin_lock_bh(&np->rx_lock);
 	spin_lock_irq(&np->tx_lock);
@@ -1710,9 +1702,6 @@ static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * data)
 
 static const struct ethtool_ops xennet_ethtool_ops =
 {
-	.set_tx_csum = ethtool_op_set_tx_csum,
-	.set_sg = xennet_set_sg,
-	.set_tso = xennet_set_tso,
 	.get_link = ethtool_op_get_link,
 
 	.get_sset_count = xennet_get_sset_count,
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH] veth: convert to hw_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller

This should probably get TSO available as it's basically a loopback device.
Offloads are left disabled by default - as before.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/veth.c |   45 +++++----------------------------------------
 1 files changed, 5 insertions(+), 40 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 2de9b90..6542288 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -36,7 +36,6 @@ struct veth_net_stats {
 struct veth_priv {
 	struct net_device *peer;
 	struct veth_net_stats __percpu *stats;
-	unsigned ip_summed;
 };
 
 /*
@@ -99,47 +98,10 @@ static void veth_get_ethtool_stats(struct net_device *dev,
 	data[0] = priv->peer->ifindex;
 }
 
-static u32 veth_get_rx_csum(struct net_device *dev)
-{
-	struct veth_priv *priv;
-
-	priv = netdev_priv(dev);
-	return priv->ip_summed == CHECKSUM_UNNECESSARY;
-}
-
-static int veth_set_rx_csum(struct net_device *dev, u32 data)
-{
-	struct veth_priv *priv;
-
-	priv = netdev_priv(dev);
-	priv->ip_summed = data ? CHECKSUM_UNNECESSARY : CHECKSUM_NONE;
-	return 0;
-}
-
-static u32 veth_get_tx_csum(struct net_device *dev)
-{
-	return (dev->features & NETIF_F_NO_CSUM) != 0;
-}
-
-static int veth_set_tx_csum(struct net_device *dev, u32 data)
-{
-	if (data)
-		dev->features |= NETIF_F_NO_CSUM;
-	else
-		dev->features &= ~NETIF_F_NO_CSUM;
-	return 0;
-}
-
 static const struct ethtool_ops veth_ethtool_ops = {
 	.get_settings		= veth_get_settings,
 	.get_drvinfo		= veth_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
-	.get_rx_csum		= veth_get_rx_csum,
-	.set_rx_csum		= veth_set_rx_csum,
-	.get_tx_csum		= veth_get_tx_csum,
-	.set_tx_csum		= veth_set_tx_csum,
-	.get_sg			= ethtool_op_get_sg,
-	.set_sg			= ethtool_op_set_sg,
 	.get_strings		= veth_get_strings,
 	.get_sset_count		= veth_get_sset_count,
 	.get_ethtool_stats	= veth_get_ethtool_stats,
@@ -168,8 +130,9 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* don't change ip_summed == CHECKSUM_PARTIAL, as that
 	   will cause bad checksum on forwarded packets */
-	if (skb->ip_summed == CHECKSUM_NONE)
-		skb->ip_summed = rcv_priv->ip_summed;
+	if (skb->ip_summed == CHECKSUM_NONE &&
+	    rcv->features & NETIF_F_RXCSUM)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
 	length = skb->len;
 	if (dev_forward_skb(rcv, skb) != NET_RX_SUCCESS)
@@ -304,6 +267,8 @@ static void veth_setup(struct net_device *dev)
 	dev->ethtool_ops = &veth_ethtool_ops;
 	dev->features |= NETIF_F_LLTX;
 	dev->destructor = veth_dev_free;
+
+	dev->hw_features = NETIF_F_NO_CSUM | NETIF_F_SG | NETIF_F_RXCSUM;
 }
 
 /*
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH] net: convert sunhme/sungem network drivers to hw_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller

Side effects:
 - TX offloads (HW csum, scatter-gather) can be toggled now
 - RX checksum is reported correctly now (it's always active)

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
[Trivial conversion, or rather: extension in this case]

 drivers/net/sungem.c |    3 ++-
 drivers/net/sunhme.c |    6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
index c1a3448..a935426 100644
--- a/drivers/net/sungem.c
+++ b/drivers/net/sungem.c
@@ -3146,7 +3146,8 @@ static int __devinit gem_init_one(struct pci_dev *pdev,
 			    gp->phy_mii.def ? gp->phy_mii.def->name : "no");
 
 	/* GEM can do it all... */
-	dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_LLTX;
+	dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM;
+	dev->features |= dev->hw_features | NETIF_F_RXCSUM | NETIF_F_LLTX;
 	if (pci_using_dac)
 		dev->features |= NETIF_F_HIGHDMA;
 
diff --git a/drivers/net/sunhme.c b/drivers/net/sunhme.c
index eb4f59f..80e907d 100644
--- a/drivers/net/sunhme.c
+++ b/drivers/net/sunhme.c
@@ -2788,7 +2788,8 @@ static int __devinit happy_meal_sbus_probe_one(struct platform_device *op, int i
 	dev->ethtool_ops = &hme_ethtool_ops;
 
 	/* Happy Meal can do it all... */
-	dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM;
+	dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM;
+	dev->features |= dev->hw_features | NETIF_F_RXCSUM;
 
 	dev->irq = op->archdata.irqs[0];
 
@@ -3113,7 +3114,8 @@ static int __devinit happy_meal_pci_probe(struct pci_dev *pdev,
 	dev->dma = 0;
 
 	/* Happy Meal can do it all... */
-	dev->features |= NETIF_F_SG | NETIF_F_HW_CSUM;
+	dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM;
+	dev->features |= dev->hw_features | NETIF_F_RXCSUM;
 
 #if defined(CONFIG_SBUS) && defined(CONFIG_PCI)
 	/* Hook up PCI register/descriptor accessors. */
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH] skge: convert to hw_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger

>From the code it looks like the hardware might support full HW_CSUM not
just IP_CSUM.  This needs testing and so is not changed here.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/skge.c |   53 ++++-----------------------------------------------
 drivers/net/skge.h |    1 -
 2 files changed, 5 insertions(+), 49 deletions(-)

[Simple conversion to hw_features, where constraints can be
detected at device init time.]

diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 35b28f4..0082808 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -537,46 +537,6 @@ static int skge_nway_reset(struct net_device *dev)
 	return 0;
 }
 
-static int skge_set_sg(struct net_device *dev, u32 data)
-{
-	struct skge_port *skge = netdev_priv(dev);
-	struct skge_hw *hw = skge->hw;
-
-	if (hw->chip_id == CHIP_ID_GENESIS && data)
-		return -EOPNOTSUPP;
-	return ethtool_op_set_sg(dev, data);
-}
-
-static int skge_set_tx_csum(struct net_device *dev, u32 data)
-{
-	struct skge_port *skge = netdev_priv(dev);
-	struct skge_hw *hw = skge->hw;
-
-	if (hw->chip_id == CHIP_ID_GENESIS && data)
-		return -EOPNOTSUPP;
-
-	return ethtool_op_set_tx_csum(dev, data);
-}
-
-static u32 skge_get_rx_csum(struct net_device *dev)
-{
-	struct skge_port *skge = netdev_priv(dev);
-
-	return skge->rx_csum;
-}
-
-/* Only Yukon supports checksum offload. */
-static int skge_set_rx_csum(struct net_device *dev, u32 data)
-{
-	struct skge_port *skge = netdev_priv(dev);
-
-	if (skge->hw->chip_id == CHIP_ID_GENESIS && data)
-		return -EOPNOTSUPP;
-
-	skge->rx_csum = data;
-	return 0;
-}
-
 static void skge_get_pauseparam(struct net_device *dev,
 				struct ethtool_pauseparam *ecmd)
 {
@@ -925,10 +885,6 @@ static const struct ethtool_ops skge_ethtool_ops = {
 	.set_pauseparam = skge_set_pauseparam,
 	.get_coalesce	= skge_get_coalesce,
 	.set_coalesce	= skge_set_coalesce,
-	.set_sg		= skge_set_sg,
-	.set_tx_csum	= skge_set_tx_csum,
-	.get_rx_csum	= skge_get_rx_csum,
-	.set_rx_csum	= skge_set_rx_csum,
 	.get_strings	= skge_get_strings,
 	.phys_id	= skge_phys_id,
 	.get_sset_count = skge_get_sset_count,
@@ -3085,7 +3041,8 @@ static struct sk_buff *skge_rx_get(struct net_device *dev,
 	}
 
 	skb_put(skb, len);
-	if (skge->rx_csum) {
+
+	if (dev->features & NETIF_F_RXCSUM) {
 		skb->csum = csum;
 		skb->ip_summed = CHECKSUM_COMPLETE;
 	}
@@ -3847,10 +3804,10 @@ static struct net_device *skge_devinit(struct skge_hw *hw, int port,
 	setup_timer(&skge->link_timer, xm_link_timer, (unsigned long) skge);
 
 	if (hw->chip_id != CHIP_ID_GENESIS) {
-		dev->features |= NETIF_F_IP_CSUM | NETIF_F_SG;
-		skge->rx_csum = 1;
+		dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG |
+		                   NETIF_F_RXCSUM;
+		dev->features |= dev->hw_features;
 	}
-	dev->features |= NETIF_F_GRO;
 
 	/* read the mac address */
 	memcpy_fromio(dev->dev_addr, hw->regs + B2_MAC_1 + port*8, ETH_ALEN);
diff --git a/drivers/net/skge.h b/drivers/net/skge.h
index 507addc..f055c47 100644
--- a/drivers/net/skge.h
+++ b/drivers/net/skge.h
@@ -2460,7 +2460,6 @@ struct skge_port {
 	struct timer_list    link_timer;
 	enum pause_control   flow_control;
 	enum pause_status    flow_status;
-	u8		     rx_csum;
 	u8		     blink_on;
 	u8		     wol;
 	u8		     autoneg;	/* AUTONEG_ENABLE, AUTONEG_DISABLE */
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH] jme: convert offload constraints to ndo_fix_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev; +Cc: Guo-Fu Tseng

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/jme.c |   77 +++++++++++++----------------------------------------
 drivers/net/jme.h |    2 -
 2 files changed, 19 insertions(+), 60 deletions(-)

This is an example conversion to hw_features and friends for MTU-based
offload disabling.

If there are some changes needed, I'll leave that to you as I don't have
the hardware to test this on.

diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 994c809..be4773f 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -2230,17 +2230,9 @@ jme_change_mtu(struct net_device *netdev, int new_mtu)
 		jme_restart_rx_engine(jme);
 	}
 
-	if (new_mtu > 1900) {
-		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_IP_CSUM | NETIF_F_IPV6_CSUM;
-		if (test_bit(JME_FLAG_TSO, &jme->flags))
-			netdev->features |= NETIF_F_TSO | NETIF_F_TSO6;
-	}
-
 	netdev->mtu = new_mtu;
+	netdev_update_features(netdev);
+
 	jme_reset_link(jme);
 
 	return 0;
@@ -2640,19 +2632,20 @@ jme_set_msglevel(struct net_device *netdev, u32 value)
 }
 
 static u32
-jme_get_rx_csum(struct net_device *netdev)
+jme_fix_features(struct net_device *netdev, u32 features)
 {
-	struct jme_adapter *jme = netdev_priv(netdev);
-	return jme->reg_rxmcs & RXMCS_CHECKSUM;
+	if (netdev->mtu > 1900)
+		features &= ~(NETIF_F_ALL_TSO | NETIF_F_ALL_CSUM);
+	return features;
 }
 
 static int
-jme_set_rx_csum(struct net_device *netdev, u32 on)
+jme_set_features(struct net_device *netdev, u32 features)
 {
 	struct jme_adapter *jme = netdev_priv(netdev);
 
 	spin_lock_bh(&jme->rxmcs_lock);
-	if (on)
+	if (features & NETIF_F_RXCSUM)
 		jme->reg_rxmcs |= RXMCS_CHECKSUM;
 	else
 		jme->reg_rxmcs &= ~RXMCS_CHECKSUM;
@@ -2663,42 +2656,6 @@ jme_set_rx_csum(struct net_device *netdev, u32 on)
 }
 
 static int
-jme_set_tx_csum(struct net_device *netdev, u32 on)
-{
-	struct jme_adapter *jme = netdev_priv(netdev);
-
-	if (on) {
-		set_bit(JME_FLAG_TXCSUM, &jme->flags);
-		if (netdev->mtu <= 1900)
-			netdev->features |=
-				NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
-	} else {
-		clear_bit(JME_FLAG_TXCSUM, &jme->flags);
-		netdev->features &=
-				~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
-	}
-
-	return 0;
-}
-
-static int
-jme_set_tso(struct net_device *netdev, u32 on)
-{
-	struct jme_adapter *jme = netdev_priv(netdev);
-
-	if (on) {
-		set_bit(JME_FLAG_TSO, &jme->flags);
-		if (netdev->mtu <= 1900)
-			netdev->features |= NETIF_F_TSO | NETIF_F_TSO6;
-	} else {
-		clear_bit(JME_FLAG_TSO, &jme->flags);
-		netdev->features &= ~(NETIF_F_TSO | NETIF_F_TSO6);
-	}
-
-	return 0;
-}
-
-static int
 jme_nway_reset(struct net_device *netdev)
 {
 	struct jme_adapter *jme = netdev_priv(netdev);
@@ -2839,11 +2796,6 @@ static const struct ethtool_ops jme_ethtool_ops = {
 	.get_link		= jme_get_link,
 	.get_msglevel           = jme_get_msglevel,
 	.set_msglevel           = jme_set_msglevel,
-	.get_rx_csum		= jme_get_rx_csum,
-	.set_rx_csum		= jme_set_rx_csum,
-	.set_tx_csum		= jme_set_tx_csum,
-	.set_tso		= jme_set_tso,
-	.set_sg			= ethtool_op_set_sg,
 	.nway_reset             = jme_nway_reset,
 	.get_eeprom_len		= jme_get_eeprom_len,
 	.get_eeprom		= jme_get_eeprom,
@@ -2903,6 +2855,8 @@ static const struct net_device_ops jme_netdev_ops = {
 	.ndo_change_mtu		= jme_change_mtu,
 	.ndo_tx_timeout		= jme_tx_timeout,
 	.ndo_vlan_rx_register	= jme_vlan_rx_register,
+	.ndo_fix_features       = jme_fix_features,
+	.ndo_set_features       = jme_set_features,
 };
 
 static int __devinit
@@ -2957,6 +2911,12 @@ 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->hw_features		=	NETIF_F_IP_CSUM |
+						NETIF_F_IPV6_CSUM |
+						NETIF_F_SG |
+						NETIF_F_TSO |
+						NETIF_F_TSO6 |
+						NETIF_F_RXCSUM;
 	netdev->features		=	NETIF_F_IP_CSUM |
 						NETIF_F_IPV6_CSUM |
 						NETIF_F_SG |
@@ -3040,8 +3000,9 @@ jme_init_one(struct pci_dev *pdev,
 	jme->reg_txpfc = 0;
 	jme->reg_pmcs = PMCS_MFEN;
 	jme->reg_gpreg1 = GPREG1_DEFAULT;
-	set_bit(JME_FLAG_TXCSUM, &jme->flags);
-	set_bit(JME_FLAG_TSO, &jme->flags);
+
+	if (jme->reg_rxmcs & RXMCS_CHECKSUM)
+		netdev->features |= NETIF_F_RXCSUM;
 
 	/*
 	 * Get Max Read Req Size from PCI Config Space
diff --git a/drivers/net/jme.h b/drivers/net/jme.h
index 8bf3045..e9aaeca 100644
--- a/drivers/net/jme.h
+++ b/drivers/net/jme.h
@@ -468,8 +468,6 @@ struct jme_adapter {
 enum jme_flags_bits {
 	JME_FLAG_MSI		= 1,
 	JME_FLAG_SSET		= 2,
-	JME_FLAG_TXCSUM		= 3,
-	JME_FLAG_TSO		= 4,
 	JME_FLAG_POLL		= 5,
 	JME_FLAG_SHUTDOWN	= 6,
 };
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH] virtio_net: convert to hw_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev; +Cc: Rusty Russell, Michael S. Tsirkin, virtualization

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/virtio_net.c |   46 +++++++++++++++++++---------------------------
 1 files changed, 19 insertions(+), 27 deletions(-)

This is an example conversion to hw_features when constraints are
constant and can be detected at init time.

Build tested only.

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 82dba5a..0cb0b06 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -710,17 +710,6 @@ static int virtnet_close(struct net_device *dev)
 	return 0;
 }
 
-static int virtnet_set_tx_csum(struct net_device *dev, u32 data)
-{
-	struct virtnet_info *vi = netdev_priv(dev);
-	struct virtio_device *vdev = vi->vdev;
-
-	if (data && !virtio_has_feature(vdev, VIRTIO_NET_F_CSUM))
-		return -ENOSYS;
-
-	return ethtool_op_set_tx_hw_csum(dev, data);
-}
-
 static void virtnet_set_rx_mode(struct net_device *dev)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
@@ -822,10 +811,6 @@ static void virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
 }
 
 static const struct ethtool_ops virtnet_ethtool_ops = {
-	.set_tx_csum = virtnet_set_tx_csum,
-	.set_sg = ethtool_op_set_sg,
-	.set_tso = ethtool_op_set_tso,
-	.set_ufo = ethtool_op_set_ufo,
 	.get_link = ethtool_op_get_link,
 };
 
@@ -912,22 +897,29 @@ static int virtnet_probe(struct virtio_device *vdev)
 	SET_NETDEV_DEV(dev, &vdev->dev);
 
 	/* Do we support "hardware" checksums? */
-	if (csum && virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
 		/* This opens up the world of extra features. */
-		dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
-		if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
-			dev->features |= NETIF_F_TSO | NETIF_F_UFO
+		dev->hw_features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+		if (csum)
+			dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+
+		if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
+			dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
 				| NETIF_F_TSO_ECN | NETIF_F_TSO6;
 		}
 		/* Individual feature bits: what can host handle? */
-		if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO4))
-			dev->features |= NETIF_F_TSO;
-		if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO6))
-			dev->features |= NETIF_F_TSO6;
-		if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
-			dev->features |= NETIF_F_TSO_ECN;
-		if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UFO))
-			dev->features |= NETIF_F_UFO;
+		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO4))
+			dev->hw_features |= NETIF_F_TSO;
+		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO6))
+			dev->hw_features |= NETIF_F_TSO6;
+		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
+			dev->hw_features |= NETIF_F_TSO_ECN;
+		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UFO))
+			dev->hw_features |= NETIF_F_UFO;
+
+		if (gso)
+			dev->features |= dev->hw_features & (NETIF_F_ALL_TSO|NETIF_F_UFO);
+		/* (!csum && gso) case will be fixed by register_netdev() */
 	}
 
 	/* Configuration may specify what MAC to use.  Otherwise random. */
-- 
1.7.2.5


^ permalink raw reply related

* [RFC PATCH] bridge: convert br_features_recompute() to ndo_fix_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev; +Cc: Stephen Hemminger, David S. Miller, Herbert Xu, Eric Dumazet,
	bridge

This is a minimal change. br_features_recompute() might be just merged
to br_fix_features() since it should not be used explicitly anymore.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/bridge/br_device.c  |   57 +++++------------------------------------------
 net/bridge/br_if.c      |   16 +++++-------
 net/bridge/br_notify.c  |    2 +-
 net/bridge/br_private.h |    3 +-
 4 files changed, 15 insertions(+), 63 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 21e5901..d6ede61 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -79,8 +79,7 @@ static int br_dev_open(struct net_device *dev)
 	struct net_bridge *br = netdev_priv(dev);
 
 	netif_carrier_off(dev);
-
-	br_features_recompute(br);
+	netdev_update_features(dev);
 	netif_start_queue(dev);
 	br_stp_enable_bridge(br);
 	br_multicast_open(br);
@@ -177,48 +176,11 @@ static void br_getinfo(struct net_device *dev, struct ethtool_drvinfo *info)
 	strcpy(info->bus_info, "N/A");
 }
 
-static int br_set_sg(struct net_device *dev, u32 data)
+static u32 br_fix_features(struct net_device *dev, u32 features)
 {
 	struct net_bridge *br = netdev_priv(dev);
 
-	if (data)
-		br->feature_mask |= NETIF_F_SG;
-	else
-		br->feature_mask &= ~NETIF_F_SG;
-
-	br_features_recompute(br);
-	return 0;
-}
-
-static int br_set_tso(struct net_device *dev, u32 data)
-{
-	struct net_bridge *br = netdev_priv(dev);
-
-	if (data)
-		br->feature_mask |= NETIF_F_TSO;
-	else
-		br->feature_mask &= ~NETIF_F_TSO;
-
-	br_features_recompute(br);
-	return 0;
-}
-
-static int br_set_tx_csum(struct net_device *dev, u32 data)
-{
-	struct net_bridge *br = netdev_priv(dev);
-
-	if (data)
-		br->feature_mask |= NETIF_F_NO_CSUM;
-	else
-		br->feature_mask &= ~NETIF_F_ALL_CSUM;
-
-	br_features_recompute(br);
-	return 0;
-}
-
-static int br_set_flags(struct net_device *netdev, u32 data)
-{
-	return ethtool_op_set_flags(netdev, data, ETH_FLAG_TXVLAN);
+	return br_features_recompute(br, features);
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -319,16 +281,6 @@ static int br_del_slave(struct net_device *dev, struct net_device *slave_dev)
 static const struct ethtool_ops br_ethtool_ops = {
 	.get_drvinfo    = br_getinfo,
 	.get_link	= ethtool_op_get_link,
-	.get_tx_csum	= ethtool_op_get_tx_csum,
-	.set_tx_csum 	= br_set_tx_csum,
-	.get_sg		= ethtool_op_get_sg,
-	.set_sg		= br_set_sg,
-	.get_tso	= ethtool_op_get_tso,
-	.set_tso	= br_set_tso,
-	.get_ufo	= ethtool_op_get_ufo,
-	.set_ufo	= ethtool_op_set_ufo,
-	.get_flags	= ethtool_op_get_flags,
-	.set_flags	= br_set_flags,
 };
 
 static const struct net_device_ops br_netdev_ops = {
@@ -347,6 +299,7 @@ static const struct net_device_ops br_netdev_ops = {
 #endif
 	.ndo_add_slave		 = br_add_slave,
 	.ndo_del_slave		 = br_del_slave,
+	.ndo_fix_features        = br_fix_features,
 };
 
 static void br_dev_free(struct net_device *dev)
@@ -371,4 +324,6 @@ void br_dev_setup(struct net_device *dev)
 	dev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
 			NETIF_F_GSO_MASK | NETIF_F_NO_CSUM | NETIF_F_LLTX |
 			NETIF_F_NETNS_LOCAL | NETIF_F_GSO | NETIF_F_HW_VLAN_TX;
+	dev->hw_features = NETIF_F_NO_CSUM | NETIF_F_SG | NETIF_F_GSO_MASK |
+			   NETIF_F_HW_VLAN_TX;
 }
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 718b603..3622052 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -205,7 +205,6 @@ static struct net_device *new_bridge_dev(struct net *net, const char *name)
 
 	memcpy(br->group_addr, br_group_address, ETH_ALEN);
 
-	br->feature_mask = dev->features;
 	br->stp_enabled = BR_NO_STP;
 	br->designated_root = br->bridge_id;
 	br->root_path_cost = 0;
@@ -364,15 +363,15 @@ int br_min_mtu(const struct net_bridge *br)
 /*
  * Recomputes features using slave's features
  */
-void br_features_recompute(struct net_bridge *br)
+u32 br_features_recompute(struct net_bridge *br, u32 features)
 {
 	struct net_bridge_port *p;
-	u32 features, mask;
+	u32 mask;
 
-	features = mask = br->feature_mask;
 	if (list_empty(&br->port_list))
-		goto done;
+		return features;
 
+	mask = features;
 	features &= ~NETIF_F_ONE_FOR_ALL;
 
 	list_for_each_entry(p, &br->port_list, list) {
@@ -380,8 +379,7 @@ void br_features_recompute(struct net_bridge *br)
 						     p->dev->features, mask);
 	}
 
-done:
-	br->dev->features = netdev_fix_features(br->dev, features);
+	return features;
 }
 
 /* called with RTNL */
@@ -448,7 +446,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
 
 	spin_lock_bh(&br->lock);
 	changed_addr = br_stp_recalculate_bridge_id(br);
-	br_features_recompute(br);
+	netdev_update_features(br->dev);
 
 	if ((dev->flags & IFF_UP) && netif_carrier_ok(dev) &&
 	    (br->dev->flags & IFF_UP))
@@ -496,7 +494,7 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 
 	spin_lock_bh(&br->lock);
 	br_stp_recalculate_bridge_id(br);
-	br_features_recompute(br);
+	netdev_update_features(br->dev);
 	spin_unlock_bh(&br->lock);
 
 	return 0;
diff --git a/net/bridge/br_notify.c b/net/bridge/br_notify.c
index 7d337c9..274ef4a 100644
--- a/net/bridge/br_notify.c
+++ b/net/bridge/br_notify.c
@@ -62,7 +62,7 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 	case NETDEV_FEAT_CHANGE:
 		spin_lock_bh(&br->lock);
 		if (netif_running(br->dev))
-			br_features_recompute(br);
+			netdev_update_features(br->dev);
 		spin_unlock_bh(&br->lock);
 		break;
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 387013d..077c351 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -182,7 +182,6 @@ struct net_bridge
 	struct br_cpu_netstats __percpu *stats;
 	spinlock_t			hash_lock;
 	struct hlist_head		hash[BR_HASH_SIZE];
-	u32				feature_mask;
 #ifdef CONFIG_BRIDGE_NETFILTER
 	struct rtable 			fake_rtable;
 	bool				nf_call_iptables;
@@ -375,7 +374,7 @@ extern int br_add_if(struct net_bridge *br,
 extern int br_del_if(struct net_bridge *br,
 	      struct net_device *dev);
 extern int br_min_mtu(const struct net_bridge *br);
-extern void br_features_recompute(struct net_bridge *br);
+extern u32 br_features_recompute(struct net_bridge *br, u32 features);
 
 /* br_input.c */
 extern int br_handle_frame_finish(struct sk_buff *skb);
-- 
1.7.2.5


^ permalink raw reply related

* [RFC PATCH] vlan: convert VLAN devices to use hw_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev
  Cc: Patrick McHardy, David S. Miller, Jesse Gross, John Fastabend,
	Eric Dumazet

Note: get_flags was actually broken, because it should return the
flags capped with vlan_features. This is now done implicitly by
limiting netdev->hw_features.

RX checksumming offload control is (and was) broken, as there was no way
before to say whether it's done for tagged packets.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 net/8021q/vlan.c     |    3 +--
 net/8021q/vlan_dev.c |   50 ++++++++++++++------------------------------------
 2 files changed, 15 insertions(+), 38 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 7850412..4b575de 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -329,8 +329,7 @@ static void vlan_transfer_features(struct net_device *dev,
 {
 	u32 old_features = vlandev->features;
 
-	vlandev->features &= ~dev->vlan_features;
-	vlandev->features |= dev->features & dev->vlan_features;
+	netdev_update_features(vlandev);
 	vlandev->gso_max_size = dev->gso_max_size;
 
 	if (dev->features & NETIF_F_HW_VLAN_TX)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index e34ea9e..b84a46b 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -704,8 +704,8 @@ static int vlan_dev_init(struct net_device *dev)
 					  (1<<__LINK_STATE_DORMANT))) |
 		      (1<<__LINK_STATE_PRESENT);
 
-	dev->features |= real_dev->features & real_dev->vlan_features;
-	dev->features |= NETIF_F_LLTX;
+	dev->hw_features = real_dev->vlan_features & NETIF_F_ALL_TX_OFFLOADS;
+	dev->features |= real_dev->vlan_features | NETIF_F_LLTX;
 	dev->gso_max_size = real_dev->gso_max_size;
 
 	/* ipv6 shared card related stuff */
@@ -759,6 +759,17 @@ static void vlan_dev_uninit(struct net_device *dev)
 	}
 }
 
+static u32 vlan_dev_fix_features(struct net_device *dev, u32 features)
+{
+	struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
+
+	features &= (real_dev->features | NETIF_F_LLTX);
+	if (dev_ethtool_get_rx_csum(real_dev))
+		features |= NETIF_F_RXCSUM;
+
+	return features;
+}
+
 static int vlan_ethtool_get_settings(struct net_device *dev,
 				     struct ethtool_cmd *cmd)
 {
@@ -774,18 +785,6 @@ static void vlan_ethtool_get_drvinfo(struct net_device *dev,
 	strcpy(info->fw_version, "N/A");
 }
 
-static u32 vlan_ethtool_get_rx_csum(struct net_device *dev)
-{
-	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_rx_csum(vlan->real_dev);
-}
-
-static u32 vlan_ethtool_get_flags(struct net_device *dev)
-{
-	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_flags(vlan->real_dev);
-}
-
 static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 {
 
@@ -823,32 +822,10 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, st
 	return stats;
 }
 
-static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
-{
-       if (data) {
-		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
-
-		/* Underlying device must support TSO for VLAN-tagged packets
-		 * and must have TSO enabled now.
-		 */
-		if (!(real_dev->vlan_features & NETIF_F_TSO))
-			return -EOPNOTSUPP;
-		if (!(real_dev->features & NETIF_F_TSO))
-			return -EINVAL;
-		dev->features |= NETIF_F_TSO;
-	} else {
-		dev->features &= ~NETIF_F_TSO;
-	}
-	return 0;
-}
-
 static const struct ethtool_ops vlan_ethtool_ops = {
 	.get_settings	        = vlan_ethtool_get_settings,
 	.get_drvinfo	        = vlan_ethtool_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
-	.get_rx_csum		= vlan_ethtool_get_rx_csum,
-	.get_flags		= vlan_ethtool_get_flags,
-	.set_tso                = vlan_ethtool_set_tso,
 };
 
 static const struct net_device_ops vlan_netdev_ops = {
@@ -874,6 +851,7 @@ static const struct net_device_ops vlan_netdev_ops = {
 	.ndo_fcoe_get_wwn	= vlan_dev_fcoe_get_wwn,
 	.ndo_fcoe_ddp_target	= vlan_dev_fcoe_ddp_target,
 #endif
+	.ndo_fix_features	= vlan_dev_fix_features,
 };
 
 void vlan_setup(struct net_device *dev)
-- 
1.7.2.5


^ permalink raw reply related

* [RFC PATCH] bonding: convert bond_compute_features() to ndo_fix/set_features
From: Michał Mirosław @ 2011-03-31 11:01 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Jiri Pirko,
	Neil Horman

Note: Updating of vlan_features is and was broken, as its not propagated
to vlan devices already created ontop of the bond.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/bonding/bond_main.c |   45 +++++++++++++++++++++++++-------------
 1 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 16d6fe9..73d2661 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1396,29 +1396,42 @@ static int bond_sethwaddr(struct net_device *bond_dev,
  * feature bits are managed elsewhere, so preserve those feature bits
  * on the master device.
  */
-static int bond_compute_features(struct bonding *bond)
+static u32 bond_fix_features(struct net_device *bond_dev, u32 features)
 {
+	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
-	struct net_device *bond_dev = bond->dev;
-	u32 features = bond_dev->features;
-	u32 vlan_features = 0;
-	unsigned short max_hard_header_len = max((u16)ETH_HLEN,
-						bond_dev->hard_header_len);
 	int i;
 
 	features &= ~(NETIF_F_ALL_CSUM | BOND_VLAN_FEATURES);
 	features |=  NETIF_F_GSO_MASK | NETIF_F_NO_CSUM;
 
 	if (!bond->first_slave)
-		goto done;
+		return features;
 
 	features &= ~NETIF_F_ONE_FOR_ALL;
 
-	vlan_features = bond->first_slave->dev->vlan_features;
 	bond_for_each_slave(bond, slave, i) {
 		features = netdev_increment_features(features,
 						     slave->dev->features,
 						     NETIF_F_ONE_FOR_ALL);
+	}
+
+	return features;
+}
+
+static int bond_set_features(struct net_device *bond_dev, u32 features)
+{
+	struct bonding *bond = netdev_priv(bond_dev);
+	struct slave *slave;
+	unsigned short max_hard_header_len = ETH_HLEN;
+	u32 vlan_features = 0;
+	int i;
+
+	if (!bond->first_slave)
+		return 0;
+
+	vlan_features = bond->first_slave->dev->vlan_features;
+	bond_for_each_slave(bond, slave, i) {
 		vlan_features = netdev_increment_features(vlan_features,
 							slave->dev->vlan_features,
 							NETIF_F_ONE_FOR_ALL);
@@ -1426,10 +1439,8 @@ static int bond_compute_features(struct bonding *bond)
 			max_hard_header_len = slave->dev->hard_header_len;
 	}
 
-done:
-	features |= (bond_dev->features & BOND_VLAN_FEATURES);
-	bond_dev->features = netdev_fix_features(bond_dev, features);
-	bond_dev->vlan_features = netdev_fix_features(bond_dev, vlan_features);
+	/* vlan_features will be trimmed by vlan devices */
+	bond_dev->vlan_features = vlan_features;
 	bond_dev->hard_header_len = max_hard_header_len;
 
 	return 0;
@@ -1757,7 +1768,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	new_slave->delay = 0;
 	new_slave->link_failure_count = 0;
 
-	bond_compute_features(bond);
+	netdev_update_features(bond_dev);
 
 	write_unlock_bh(&bond->lock);
 
@@ -2021,7 +2032,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 	/* release the slave from its bond */
 	bond_detach_slave(bond, slave);
 
-	bond_compute_features(bond);
+	netdev_update_features(bond_dev);
 
 	if (bond->primary_slave == slave)
 		bond->primary_slave = NULL;
@@ -2201,7 +2212,7 @@ static int bond_release_all(struct net_device *bond_dev)
 			bond_alb_deinit_slave(bond, slave);
 		}
 
-		bond_compute_features(bond);
+		netdev_update_features(bond_dev);
 
 		bond_destroy_slave_symlinks(bond_dev, slave_dev);
 		bond_del_vlans_from_slave(bond, slave_dev);
@@ -3380,7 +3391,7 @@ static int bond_slave_netdev_event(unsigned long event,
 		 */
 		break;
 	case NETDEV_FEAT_CHANGE:
-		bond_compute_features(bond);
+		netdev_update_features(bond_dev);
 		break;
 	default:
 		break;
@@ -4478,6 +4489,8 @@ static const struct net_device_ops bond_netdev_ops = {
 #endif
 	.ndo_add_slave		= bond_enslave,
 	.ndo_del_slave		= bond_release,
+	.ndo_fix_features	= bond_fix_features,
+	.ndo_set_features	= bond_set_features,
 };
 
 static void bond_destructor(struct net_device *bond_dev)
-- 
1.7.2.5


^ permalink raw reply related

* [PATCH] net: Fix dev dev_ethtool_get_rx_csum() for forced NETIF_F_RXCSUM
From: Michał Mirosław @ 2011-03-31  9:58 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller

dev_ethtool_get_rx_csum() won't report rx checksumming when it's not
changeable and driver is converted to hw_features and friends. Fix this.

(dev->hw_features & NETIF_F_RXCSUM) check is dropped - if the
ethtool_ops->get_rx_csum is set, then driver is not coverted, yet.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
[this is a fix also for 2.6.39]

 include/linux/netdevice.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5eeb2cd..0249fe7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2598,8 +2598,8 @@ static inline int dev_ethtool_get_settings(struct net_device *dev,
 
 static inline u32 dev_ethtool_get_rx_csum(struct net_device *dev)
 {
-	if (dev->hw_features & NETIF_F_RXCSUM)
-		return !!(dev->features & NETIF_F_RXCSUM);
+	if (dev->features & NETIF_F_RXCSUM)
+		return 1;
 	if (!dev->ethtool_ops || !dev->ethtool_ops->get_rx_csum)
 		return 0;
 	return dev->ethtool_ops->get_rx_csum(dev);
-- 
1.7.2.5


^ permalink raw reply related

* Re: [PATCH] mlx4: Fixing bad size of event queue buffer
From: David Miller @ 2011-03-31  9:52 UTC (permalink / raw)
  To: yevgenyp; +Cc: netdev
In-Reply-To: <4D9449A9.9070600@mellanox.co.il>

From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Thu, 31 Mar 2011 11:30:17 +0200

> We should reduce the number of reserved completion queues from the total
> number of entries. Since the queue size is power of two, not reducing the
> reserved entries, caused a double queue size, which may lead to allocation
> failures in some cases.
> 
> Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>

Applied.

^ permalink raw reply

* Re: [PATCH] mlx4: Fixing use after free
From: David Miller @ 2011-03-31  9:52 UTC (permalink / raw)
  To: yevgenyp; +Cc: netdev, error27
In-Reply-To: <4D944954.9000701@mellanox.co.il>

From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Thu, 31 Mar 2011 11:28:52 +0200

> 
> In case of allocation failure, tried to use the promiscuous QP
> entry that was previously freed.
> Now freeing this entry only in case we will not put it back to the list
> of promiscuous entries.
> 
> Reported-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>

Applied.

^ permalink raw reply

* Re: [PATCHv2 0/9] macb: add support for Cadence GEM
From: Jamie Iles @ 2011-03-31  9:40 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD
  Cc: Jamie Iles, Nicolas Ferre, David Miller, Russell King - ARM Linux,
	netdev, linux-arm-kernel, Andrew Victor, Peter Korsgaard
In-Reply-To: <20110324162517.GI6466@game.jcrosoft.org>

Hi Jean-Christophe,

On Thu, Mar 24, 2011 at 05:25:17PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:55 Tue 22 Mar     , Jamie Iles wrote:
> > Hi Jean-Christophe,
> > > Also, can you confirm that the module ID's that you are using to 
> > > differentiate between AT91 and AVR32 won't clash with MACB uses in 
> > > other, non-at91/avr32 chips, or that it doesn't matter?
> > 
> > If this does work, then it would be nice if we made the else path of the 
> > RMII AT91 tests also test for avr32 too so we aren't driving the USRIO 
> > pins on platforms that aren't at91 but also aren't avr32.  So something 
> > the patch below instead.
> I'm currently traveling so I can not test it yet will test it next week
> but looks good

Just wondering if you'd had chance to test.  If so, would you mind 
reposting and I'll rebase my patches on top of it.

I also have patches for checksum offloading, receive packet timestamping 
and wake-on-lan support but I'd like to get the basic GEM ones into next 
first.

Jamie

^ permalink raw reply

* [PATCH] mlx4: Fixing bad size of event queue buffer
From: Yevgeny Petrilin @ 2011-03-31  9:30 UTC (permalink / raw)
  To: davem; +Cc: netdev, yevgenyp

We should reduce the number of reserved completion queues from the total
number of entries. Since the queue size is power of two, not reducing the
reserved entries, caused a double queue size, which may lead to allocation
failures in some cases.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
---
 drivers/net/mlx4/eq.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/net/mlx4/eq.c b/drivers/net/mlx4/eq.c
index 506cfd0..1ad1f60 100644
--- a/drivers/net/mlx4/eq.c
+++ b/drivers/net/mlx4/eq.c
@@ -603,7 +603,9 @@ int mlx4_init_eq_table(struct mlx4_dev *dev)
 	}
 
 	for (i = 0; i < dev->caps.num_comp_vectors; ++i) {
-		err = mlx4_create_eq(dev, dev->caps.num_cqs + MLX4_NUM_SPARE_EQE,
+		err = mlx4_create_eq(dev, dev->caps.num_cqs -
+					  dev->caps.reserved_cqs +
+					  MLX4_NUM_SPARE_EQE,
 				     (dev->flags & MLX4_FLAG_MSI_X) ? i : 0,
 				     &priv->eq_table.eq[i]);
 		if (err) {
-- 
1.6.0.2




^ permalink raw reply related

* [PATCH] mlx4: Fixing use after free
From: Yevgeny Petrilin @ 2011-03-31  9:28 UTC (permalink / raw)
  To: davem; +Cc: netdev, yevgenyp, error27


In case of allocation failure, tried to use the promiscuous QP
entry that was previously freed.
Now freeing this entry only in case we will not put it back to the list
of promiscuous entries.

Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
---
 drivers/net/mlx4/mcg.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/mlx4/mcg.c b/drivers/net/mlx4/mcg.c
index e71372a..37150b2 100644
--- a/drivers/net/mlx4/mcg.c
+++ b/drivers/net/mlx4/mcg.c
@@ -469,7 +469,6 @@ static int remove_promisc_qp(struct mlx4_dev *dev, u8 vep_num, u8 port,
 
 	/*remove from list of promisc qps */
 	list_del(&pqp->list);
-	kfree(pqp);
 
 	/* set the default entry not to include the removed one */
 	mailbox = mlx4_alloc_cmd_mailbox(dev);
@@ -528,6 +527,8 @@ out_mailbox:
 out_list:
 	if (back_to_list)
 		list_add_tail(&pqp->list, &s_steer->promisc_qps[steer]);
+	else
+		kfree(pqp);
 out_mutex:
 	mutex_unlock(&priv->mcg_table.mutex);
 	return err;
-- 
1.6.0.2




^ permalink raw reply related

* Re: [PATCH 2/3] bonding:typo in comment
From: David Miller @ 2011-03-31  8:54 UTC (permalink / raw)
  To: panweiping3; +Cc: fubar, andy, netdev, linux-kernel
In-Reply-To: <1301553980-29096-1-git-send-email-panweiping3@gmail.com>

From: Pan Weiping <panweiping3@gmail.com>
Date: Thu, 31 Mar 2011 14:46:19 +0800

> use accumulates instead of acumulates.
> Signed-off-by: Pan Weiping <panweiping3@gmail.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH v3] net: Allow no-cache copy from user on transmit
From: Michał Mirosław @ 2011-03-31  8:52 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <alpine.DEB.2.00.1103302156270.17065@pokey.mtv.corp.google.com>

2011/3/31 Tom Herbert <therbert@google.com>:
> This patch uses __copy_from_user_nocache (from skb_copy_to_page)
> on transmit to bypass data cache for a performance improvement.
> This functionality is configurable per device using ethtool.
[...]
> diff --git a/include/net/sock.h b/include/net/sock.h
> index da0534d..74ce586 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1401,6 +1401,11 @@ static inline int skb_copy_to_page(struct sock *sk, char __user *from,
>                if (err)
>                        return err;
>                skb->csum = csum_block_add(skb->csum, csum, skb->len);
> +       } else if (sk->sk_route_caps & NETIF_F_NOCACHE_COPY) {
> +               if (!access_ok(VERIFY_READ, from, copy) ||
> +                   __copy_from_user_nocache(page_address(page) + off,
> +                                               from, copy))
> +                       return -EFAULT;
>        } else if (copy_from_user(page_address(page) + off, from, copy))
>                return -EFAULT;
>

BTW, the checks should take protocol into account. If, for example,
device has NOCACHE_COPY and CSUM_IPV4, but not CSUM_IPV6, then it will
take a hit for every IPv6 packet that needs checksumming in software.
Maybe this can be taken into account when sk_route_caps are set?

[...]
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 1a6e9eb..59b6ce9 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1407,7 +1407,7 @@ static int bond_compute_features(struct bonding *bond)
>        int i;
>
>        features &= ~(NETIF_F_ALL_CSUM | BOND_VLAN_FEATURES);
> -       features |=  NETIF_F_GSO_MASK | NETIF_F_NO_CSUM;
> +       features |=  NETIF_F_GSO_MASK | NETIF_F_NO_CSUM | NETIF_F_NOCACHE_COPY;
>
>        if (!bond->first_slave)
>                goto done;
[...]
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 0b88eba..454abfd 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5444,6 +5444,13 @@ int register_netdevice(struct net_device *dev)
>                dev->features &= ~NETIF_F_GSO;
>        }
>
> +       /* Turn no cache copy off if HW isn't doing checksum */
> +       if (!(dev->features & NETIF_F_ALL_CSUM) ||
> +           (dev->features & NETIF_F_NO_CSUM)) {
> +               dev->wanted_features &= ~NETIF_F_NOCACHE_COPY;
> +               dev->features &= ~NETIF_F_NOCACHE_COPY;
> +       }
> +
>        /* Enable GRO and NETIF_F_HIGHDMA for vlans by default,
>         * vlan_dev_init() will do the dev->features check, so these features
>         * are enabled only if supported by underlying device.

Why NO_CSUM is disabling NOCACHE_COPY? You combine those features in
bonding code anyway.

You didn't add netdev_fix_features() checks, so NOCACHE_COPY might be
enabled via ethtool after device is registered even if driver doesn't
support checksum offloads.

Best Regards,
Michał Mirosław

^ 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