Netdev List
 help / color / mirror / Atom feed
* [PATCH 2/3] net/hyperv: Add NETVSP protocol version negotiation
From: Haiyang Zhang @ 2011-12-15 21:45 UTC (permalink / raw)
  To: haiyangz, kys, davem, gregkh, linux-kernel, netdev, devel
In-Reply-To: <1323985517-1156-1-git-send-email-haiyangz@microsoft.com>

Automatically negotiate the highest protocol version mutually recognized by
both host and guest.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h |  101 ++++++++++++++++++++++++++++++++++++---
 drivers/net/hyperv/netvsc.c     |   82 +++++++++++++++++++++----------
 2 files changed, 149 insertions(+), 34 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index ff1b520..2877670 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -134,8 +134,7 @@ int rndis_filter_set_packet_filter(struct rndis_device *dev, u32 new_filter);
 #define NVSP_INVALID_PROTOCOL_VERSION	((u32)0xFFFFFFFF)
 
 #define NVSP_PROTOCOL_VERSION_1		2
-#define NVSP_MIN_PROTOCOL_VERSION	NVSP_PROTOCOL_VERSION_1
-#define NVSP_MAX_PROTOCOL_VERSION	NVSP_PROTOCOL_VERSION_1
+#define NVSP_PROTOCOL_VERSION_2		0x30002
 
 enum {
 	NVSP_MSG_TYPE_NONE = 0,
@@ -160,11 +159,36 @@ enum {
 	NVSP_MSG1_TYPE_SEND_RNDIS_PKT,
 	NVSP_MSG1_TYPE_SEND_RNDIS_PKT_COMPLETE,
 
-	/*
-	 * This should be set to the number of messages for the version with
-	 * the maximum number of messages.
-	 */
-	NVSP_NUM_MSG_PER_VERSION		= 9,
+	/* Version 2 messages */
+	NVSP_MSG2_TYPE_SEND_CHIMNEY_DELEGATED_BUF,
+	NVSP_MSG2_TYPE_SEND_CHIMNEY_DELEGATED_BUF_COMP,
+	NVSP_MSG2_TYPE_REVOKE_CHIMNEY_DELEGATED_BUF,
+
+	NVSP_MSG2_TYPE_RESUME_CHIMNEY_RX_INDICATION,
+
+	NVSP_MSG2_TYPE_TERMINATE_CHIMNEY,
+	NVSP_MSG2_TYPE_TERMINATE_CHIMNEY_COMP,
+
+	NVSP_MSG2_TYPE_INDICATE_CHIMNEY_EVENT,
+
+	NVSP_MSG2_TYPE_SEND_CHIMNEY_PKT,
+	NVSP_MSG2_TYPE_SEND_CHIMNEY_PKT_COMP,
+
+	NVSP_MSG2_TYPE_POST_CHIMNEY_RECV_REQ,
+	NVSP_MSG2_TYPE_POST_CHIMNEY_RECV_REQ_COMP,
+
+	NVSP_MSG2_TYPE_ALLOC_RXBUF,
+	NVSP_MSG2_TYPE_ALLOC_RXBUF_COMP,
+
+	NVSP_MSG2_TYPE_FREE_RXBUF,
+
+	NVSP_MSG2_TYPE_SEND_VMQ_RNDIS_PKT,
+	NVSP_MSG2_TYPE_SEND_VMQ_RNDIS_PKT_COMP,
+
+	NVSP_MSG2_TYPE_SEND_NDIS_CONFIG,
+
+	NVSP_MSG2_TYPE_ALLOC_CHIMNEY_HANDLE,
+	NVSP_MSG2_TYPE_ALLOC_CHIMNEY_HANDLE_COMP,
 };
 
 enum {
@@ -175,6 +199,7 @@ enum {
 	NVSP_STAT_PROTOCOL_TOO_OLD,
 	NVSP_STAT_INVALID_RNDIS_PKT,
 	NVSP_STAT_BUSY,
+	NVSP_STAT_PROTOCOL_UNSUPPORTED,
 	NVSP_STAT_MAX,
 };
 
@@ -359,9 +384,69 @@ union nvsp_1_message_uber {
 						send_rndis_pkt_complete;
 } __packed;
 
+
+/*
+ * Network VSP protocol version 2 messages:
+ */
+struct nvsp_2_vsc_capability {
+	union {
+		u64 data;
+		struct {
+			u64 vmq:1;
+			u64 chimney:1;
+			u64 sriov:1;
+			u64 ieee8021q:1;
+			u64 correlation_id:1;
+		};
+	};
+} __packed;
+
+struct nvsp_2_send_ndis_config {
+	u32 mtu;
+	u32 reserved;
+	struct nvsp_2_vsc_capability capability;
+} __packed;
+
+/* Allocate receive buffer */
+struct nvsp_2_alloc_rxbuf {
+	/* Allocation ID to match the allocation request and response */
+	u32 alloc_id;
+
+	/* Length of the VM shared memory receive buffer that needs to
+	 * be allocated
+	 */
+	u32 len;
+} __packed;
+
+/* Allocate receive buffer complete */
+struct nvsp_2_alloc_rxbuf_comp {
+	/* The NDIS_STATUS code for buffer allocation */
+	u32 status;
+
+	u32 alloc_id;
+
+	/* GPADL handle for the allocated receive buffer */
+	u32 gpadl_handle;
+
+	/* Receive buffer ID */
+	u64 recv_buf_id;
+} __packed;
+
+struct nvsp_2_free_rxbuf {
+	u64 recv_buf_id;
+} __packed;
+
+union nvsp_2_message_uber {
+	struct nvsp_2_send_ndis_config send_ndis_config;
+	struct nvsp_2_alloc_rxbuf alloc_rxbuf;
+	struct nvsp_2_alloc_rxbuf_comp alloc_rxbuf_comp;
+	struct nvsp_2_free_rxbuf free_rxbuf;
+} __packed;
+
 union nvsp_all_messages {
 	union nvsp_message_init_uber init_msg;
 	union nvsp_1_message_uber v1_msg;
+	union nvsp_2_message_uber v2_msg;
 } __packed;
 
 /* ALL Messages */
@@ -391,6 +476,8 @@ struct nvsp_message {
 struct netvsc_device {
 	struct hv_device *dev;
 
+	u32 nvsp_version;
+
 	atomic_t num_outstanding_sends;
 	bool destroy;
 	/*
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index bab627f..46828b4 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -28,6 +28,7 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/netdevice.h>
+#include <linux/if_ether.h>
 
 #include "hyperv_net.h"
 
@@ -260,27 +261,18 @@ exit:
 }
 
 
-static int netvsc_connect_vsp(struct hv_device *device)
+/* Negotiate NVSP protocol version */
+static int negotiate_nvsp_ver(struct hv_device *device,
+			      struct netvsc_device *net_device,
+			      struct nvsp_message *init_packet,
+			      u32 nvsp_ver)
 {
 	int ret, t;
-	struct netvsc_device *net_device;
-	struct nvsp_message *init_packet;
-	int ndis_version;
-	struct net_device *ndev;
-
-	net_device = get_outbound_net_device(device);
-	if (!net_device)
-		return -ENODEV;
-	ndev = net_device->ndev;
-
-	init_packet = &net_device->channel_init_pkt;
 
 	memset(init_packet, 0, sizeof(struct nvsp_message));
 	init_packet->hdr.msg_type = NVSP_MSG_TYPE_INIT;
-	init_packet->msg.init_msg.init.min_protocol_ver =
-		NVSP_MIN_PROTOCOL_VERSION;
-	init_packet->msg.init_msg.init.max_protocol_ver =
-		NVSP_MAX_PROTOCOL_VERSION;
+	init_packet->msg.init_msg.init.min_protocol_ver = nvsp_ver;
+	init_packet->msg.init_msg.init.max_protocol_ver = nvsp_ver;
 
 	/* Send the init request */
 	ret = vmbus_sendpacket(device->channel, init_packet,
@@ -290,26 +282,62 @@ static int netvsc_connect_vsp(struct hv_device *device)
 			       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 
 	if (ret != 0)
-		goto cleanup;
+		return ret;
 
 	t = wait_for_completion_timeout(&net_device->channel_init_wait, 5*HZ);
 
-	if (t == 0) {
-		ret = -ETIMEDOUT;
-		goto cleanup;
-	}
+	if (t == 0)
+		return -ETIMEDOUT;
 
 	if (init_packet->msg.init_msg.init_complete.status !=
-	    NVSP_STAT_SUCCESS) {
-		ret = -EINVAL;
-		goto cleanup;
-	}
+	    NVSP_STAT_SUCCESS)
+		return -EINVAL;
 
-	if (init_packet->msg.init_msg.init_complete.
-	    negotiated_protocol_ver != NVSP_PROTOCOL_VERSION_1) {
+	if (nvsp_ver != NVSP_PROTOCOL_VERSION_2)
+		return 0;
+
+	/* NVSPv2 only: Send NDIS config */
+	memset(init_packet, 0, sizeof(struct nvsp_message));
+	init_packet->hdr.msg_type = NVSP_MSG2_TYPE_SEND_NDIS_CONFIG;
+	init_packet->msg.v2_msg.send_ndis_config.mtu = ETH_DATA_LEN;
+
+	ret = vmbus_sendpacket(device->channel, init_packet,
+				sizeof(struct nvsp_message),
+				(unsigned long)init_packet,
+				VM_PKT_DATA_INBAND, 0);
+
+	return ret;
+}
+
+static int netvsc_connect_vsp(struct hv_device *device)
+{
+	int ret;
+	struct netvsc_device *net_device;
+	struct nvsp_message *init_packet;
+	int ndis_version;
+	struct net_device *ndev;
+
+	net_device = get_outbound_net_device(device);
+	if (!net_device)
+		return -ENODEV;
+	ndev = net_device->ndev;
+
+	init_packet = &net_device->channel_init_pkt;
+
+	/* Negotiate the latest NVSP protocol supported */
+	if (negotiate_nvsp_ver(device, net_device, init_packet,
+			       NVSP_PROTOCOL_VERSION_2) == 0) {
+		net_device->nvsp_version = NVSP_PROTOCOL_VERSION_2;
+	} else if (negotiate_nvsp_ver(device, net_device, init_packet,
+				    NVSP_PROTOCOL_VERSION_1) == 0) {
+		net_device->nvsp_version = NVSP_PROTOCOL_VERSION_1;
+	} else {
 		ret = -EPROTO;
 		goto cleanup;
 	}
+
+	pr_debug("Negotiated NVSP version:%x\n", net_device->nvsp_version);
+
 	/* Send the ndis version */
 	memset(init_packet, 0, sizeof(struct nvsp_message));
 
-- 
1.7.4.1

^ permalink raw reply related

* UPGRADE uw webmail
From: G.O. Sars @ 2011-12-15 21:08 UTC (permalink / raw)





UPGRADE uw webmail

Uw e-mail box is bijna vol.
23 GB 20 GB

De mailbox is overschreden de limiet van de
20GB, zoals gedefinieerd door uw beheerder, U bent nu
Werken bij 20.9GB,

Dat is misschien niet kunt verzenden of ontvangen van berichten zijn tot je upgrade van uw account.

Klik op de link hieronder om uw account bij te werken.
Bijj de link geeft je probleem, kopieer het en het verleden
het op uw nieuwe browser om uw account te upgraden.

===> http://sertecautomacao.com.br/contatos/use/finson/form1.html

Een bevestiging link naar u worden verzonden zodra
Wij ontvangen uw reactie.

OPMERKING: Als u niet in staat zijn opnieuw te valideren uw mailbox,Rekening zal worden uitgeschakeld!

Webmail Support Team
@ 2011
192.168.0.1

^ permalink raw reply

* Re: iwlagn regression in v3.1.5
From: Arkadiusz Miśkiewicz @ 2011-12-15 22:12 UTC (permalink / raw)
  To: wwguy
  Cc: Andrej Gelenberg, Udo Steinberg, Intel Linux Wireless,
	John W. Linville, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1323807327.7144.6.camel@wwguy-ubuntu>

On Tue, Dec 13, 2011 at 9:15 PM, wwguy <wey-yi.w.guy@intel.com> wrote:
> Hi Andrej,
>
> Could you try this patch and see any differences.

I had a problem with 3.1.5 (3.1.4 was fine) and current Linus git
where I was able to associate with my AP (5GHz) but only icmp was
working and while trying to ssh then all packets (including icmp ones)
started to be dropped.

My card is 5350 AGN [Echo Peak] and this patch makes things working
for me on top of current Linus git, too.

-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

^ permalink raw reply

* Re: [PATCH v2] sctp: fix incorrect overflow check on autoclose
From: Xi Wang @ 2011-12-15 22:13 UTC (permalink / raw)
  To: Vlad Yasevich
  Cc: netdev, linux-sctp, Andrew Morton, Andrei Pelinescu-Onciul,
	David S. Miller
In-Reply-To: <4EEA6176.9020704@hp.com>

On Dec 15, 2011, at 4:07 PM, Vlad Yasevich wrote:
> I think it would be better to keep this value in seconds and get rid
> of division in the setsockopt code.  We could then have a min and max
> values where max value could be something like 2 days.  I really don't
> see an autoclose value that is bigger then that being very useful.  In
> fact, most of the time these values are very small as one wants to close
> out idle associations.

Now I start to think exposing a new sysctl option might be a little
overkill since autoclose is often small.

How about this?

1) Simply store autoclose in seconds in setsockopt.

2) Avoid overflow in associola.c.

	asoc->timeouts[SCTP_EVENT_TIMEOUT_AUTOCLOSE] =
		(sp->autoclose > MAX_SCHEDULE_TIMEOUT / HZ)
		? MAX_SCHEDULE_TIMEOUT
		: (unsigned long)sp->autoclose * HZ;

Or we could use INT_MAX instead of MAX_SCHEDULE_TIMEOUT if you want to
keep that value consistent across 32/64 bits.

- xi

^ permalink raw reply

* Re: [PATCH 2/2] Explicitly call tcp creation and init from memcontrol.c
From: KAMEZAWA Hiroyuki @ 2011-12-15 22:20 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Glauber Costa, davem, linux-kernel, netdev, cgroups, Eric Dumazet,
	Stephen Rothwell
In-Reply-To: <20111216011316.8d58bc8f.kamezawa.hiroyu@jp.fujitsu.com>

On Fri, 16 Dec 2011 01:13:16 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Thu, 15 Dec 2011 13:34:32 +0400
> Glauber Costa <glommer@parallels.com> wrote:
> 
> > Walking the proto_list holds a read_lock, which prevents us from doing
> > allocations. Splitting the tcp create function into create + init is
> > good, but it is not enough since create_files will do allocations as well
> > (dentry ones, mostly).
> > 
> > Since this does not involve any protocol state, I propose we call the tcp
> > functions explicitly from memcontrol.c
> > 
> > With this, we lose by now the ability of doing cgroup memcontrol for
> > protocols that are loaded as modules. But at least the ones I have in mind
> > won't really need it (tcp_ipv6 being the only one, but it uses the same data
> > structures as tcp_ipv4). So I believe this to be the simpler solution to this
> > problem.
> > 
> > Signed-off-by: Glauber Costa <glommer@parallels.com>
> > CC: Hiroyouki Kamezawa <kamezawa.hiroyu@jp.fujitsu.com>
> > CC: David S. Miller <davem@davemloft.net>
> > CC: Eric Dumazet <eric.dumazet@gmail.com>
> > CC: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> Could you remake the patch onto the 'latest' linux-next ?
> As Dave mentioned, some bandaids are already applied and this patch hunks.

Applied patches by hand and did small test for hours.
seems good.

Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

^ permalink raw reply

* Re: twice past the taps, thence out to net?
From: Rick Jones @ 2011-12-15 22:22 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Stephen Hemminger, Vijay Subramanian, tcpdump-workers, netdev
In-Reply-To: <1323975606.2769.24.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

On 12/15/2011 11:00 AM, Eric Dumazet wrote:
>> Device's work better if the driver proactively manages stop_queue/wake_queue.
>> Old devices used TX_BUSY, but newer devices tend to manage the queue
>> themselves.
>>
>
> Some 'new' drivers like igb can be fooled in case skb is gso segmented ?
>
> Because igb_xmit_frame_ring() needs skb_shinfo(skb)->nr_frags + 4
> descriptors, igb should stop its queue not at MAX_SKB_FRAGS + 4, but
> MAX_SKB_FRAGS*4
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 89d576c..989da36 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4370,7 +4370,7 @@ netdev_tx_t igb_xmit_frame_ring(struct sk_buff *skb,
>   	igb_tx_map(tx_ring, first, hdr_len);
>
>   	/* Make sure there is space in the ring for the next send. */
> -	igb_maybe_stop_tx(tx_ring, MAX_SKB_FRAGS + 4);
> +	igb_maybe_stop_tx(tx_ring, MAX_SKB_FRAGS * 4);
>
>   	return NETDEV_TX_OK;


Is there a minimum transmit queue length here?  I get the impression 
that MAX_SKB_FRAGS is at least 16 and is 18 on a system with 4096 byte 
pages.  The previous addition then would be OK so long as the TX queue 
was always at least 22 entries in size, but now it would have to always 
be at least 72?

I guess things are "OK" at the moment:

raj@tardy:~/net-next/drivers/net/ethernet/intel/igb$ grep IGB_MIN_TXD *.[ch]
igb_ethtool.c:	new_tx_count = max_t(u16, new_tx_count, IGB_MIN_TXD);
igb.h:#define IGB_MIN_TXD                       80

but is that getting a little close?

rick jones

^ permalink raw reply

* Re: [PATCH 2/2] Explicitly call tcp creation and init from memcontrol.c
From: David Miller @ 2011-12-15 22:44 UTC (permalink / raw)
  To: glommer-bzQdu9zFT3WakBO8gow8eQ
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
	kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, sfr-3FnU+UHB4dNDw9hX6IcOSA
In-Reply-To: <4EEA6288.1080405-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

From: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Date: Fri, 16 Dec 2011 01:11:36 +0400

> How about the following patch then ? I am keeping protocols that has
> the cgroup stuff on in a separate list, that can be protected by a
> mutex. Therefore we can allocate without going into troubles.

This is a significantly better approach, please test and submit this
patch formally.

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

^ permalink raw reply

* Re: pull request: wireless 2011-12-15
From: David Miller @ 2011-12-15 22:54 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20111215191523.GA2561@tuxdriver.com>

From: "John W. Linville" <linville@tuxdriver.com>
Date: Thu, 15 Dec 2011 14:15:24 -0500

> Here are a few more fixes intended for the 3.2 release.  They are
> all small and narrowly focused.
> 
> First is a one liner fix for a signedness problem in NFC that could
> lead to an incorrect return code being propogated.  Next are two
> iwlwifi regression fixes that received some recent attention on the
> linux-wireless mailing list.  A third iwlwifi fix corrects a sequence
> numbering error, which can cause a lot of bad behaviour in the wireless
> network.  The ath9k fix corrects a potential for an array underrun.
> Finally, the mwifiex fix corrects a double list deletion.
> 
> Please let me know if there are problems!

Pulled, thanks John.

^ permalink raw reply

* Re: [net-next RFC PATCH 0/5] Series short description
From: Rusty Russell @ 2011-12-15 23:12 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: krkumar2, kvm, mst, netdev, virtualization, levinsasha928, davem
In-Reply-To: <1323913005.2753.47.camel@bwh-desktop>

On Thu, 15 Dec 2011 01:36:44 +0000, Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Fri, 2011-12-09 at 16:01 +1030, Rusty Russell wrote:
> > On Wed, 7 Dec 2011 17:02:04 +0000, Ben Hutchings <bhutchings@solarflare.com> wrote:
> > > Most multi-queue controllers could support a kind of hash-based
> > > filtering for TCP/IP by adjusting the RSS indirection table.  However,
> > > this table is usually quite small (64-256 entries).  This means that
> > > hash collisions will be quite common and this can result in reordering.
> > > The same applies to the small table Jason has proposed for virtio-net.
> > 
> > But this happens on real hardware today.  Better that real hardware is
> > nice, but is it overkill?
> 
> What do you mean, it happens on real hardware today?  So far as I know,
> the only cases where we have dynamic adjustment of flow steering are in
> ixgbe (big table of hash filters, I think) and sfc (perfect filters).
> I don't think that anyone's currently doing flow steering with the RSS
> indirection table.  (At least, not on Linux.  I think that Microsoft was
> intending to do so on Windows, but I don't know whether they ever did.)

Thanks, I missed the word "could".

> > And can't you reorder even with perfect matching, since prior packets
> > will be on the old queue and more recent ones on the new queue?  Does it
> > discard or requeue old ones?  Or am I missing a trick?
> 
> Yes, that is possible.  RFS is careful to avoid such reordering by only
> changing the steering of a flow when none of its packets can be in a
> software receive queue.  It is not generally possible to do the same for
> hardware receive queues.  However, when the first condition is met it is
> likely that there won't be a whole lot of packets for that flow in the
> hardware receive queue either.  (But if there are, then I think as a
> side-effect of commit 09994d1 RFS will repeatedly ask the driver to
> steer the flow.  Which isn't ideal.)

Should be easy to test, but the question is, how hard should we fight to
maintain ordering?  Dave?

It comes down to this.  We can say in the spec that a virtio nic which
offers VIRTIO_F_NET_RFS:

1) Must do a perfect matching, with perfect ordering.  This means you need
   perfect filters, and handle inter-queue ordering if you change a
   filter (requeue packets?)
2) Must do a perfect matching, but don't worry about ordering across changes.
3) Best effort matching, with perfect ordering.
3) Best effort matching, best effort ordering.

For a perfect filtering setup, the virtio nic needs to either say how
many filter slots it has, or have a way to fail an RFS request.  For
best effort, you can simply ignore RFS requests or accept hash
collisions, without bothering the guest driver at all.

Thanks,
Rusty.

^ permalink raw reply

* Re: [PATCH net-next 6/6] tg3: Make the RSS indir tbl admin configurable
From: Matt Carlson @ 2011-12-15 23:37 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Matthew Carlson, davem@davemloft.net, netdev@vger.kernel.org,
	Michael Chan
In-Reply-To: <1323983627.2773.18.camel@bwh-desktop>

On Thu, Dec 15, 2011 at 01:13:47PM -0800, Ben Hutchings wrote:
> On Thu, 2011-12-15 at 13:03 -0800, Matt Carlson wrote:
> > On Wed, Dec 14, 2011 at 01:50:59PM -0800, Ben Hutchings wrote:
> > > On Wed, 2011-12-14 at 13:10 -0800, Matt Carlson wrote:
> [...]
> > > > +	if (!indir->size) {
> > > > +		indir->size = TG3_RSS_INDIR_TBL_SIZE;
> > > > +		return 0;
> > > > +	}
> > > > +
> > > > +	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> > > > +		return -EINVAL;
> > > 
> > > This is enough to make the ethtool command work, but you're really
> > > supposed to copy min(indir->size, TG3_RSS_INDIR_TBL_SIZE) entries.
> > 
> > Could you elaborate on this?  I'm confused because I can't figure out
> > how returning half of an indirection table could be useful.
> 
> It's a generalisation of the zero-length and full-length cases.  But no,
> it isn't very useful, nor did I actually specify that anywhere!
> 
> Maybe there should be a driver operation to get the table size, and then
> the core can make sure that drivers only ever deal with full-table
> buffers.  Though that wouldn't cover your reset-to-default case.

That's how your current implementation of ethtool works, doesn't it?  It
first makes a call to get_rxfh_indir() with a zero size parameter.  The
driver interprets this as a query for the size of the driver's
indirection table.  Subsequent calls to get_rxfh_indir() with a non-zero
size are interpreted as a request for the indirection table data.

Right now, I coded get_rxfh_indir() and set_rxfh_indir() to be strict in
what they accept.  The restrictions can be relaxed though if we can
outline what the driver should do with partial tables.  (I wonder if
these types of interpretations are better placed above the driver
though.  Then again, maybe not.  Tg3 devices can tune the size of the
indirection table by powers of two if needed at the cost of a reset.)

But IMO all these ideas make the reset-to-default case more desirable.

^ permalink raw reply

* Re: [PATCH net-next 6/6] tg3: Make the RSS indir tbl admin configurable
From: Ben Hutchings @ 2011-12-15 23:43 UTC (permalink / raw)
  To: Matt Carlson; +Cc: davem@davemloft.net, netdev@vger.kernel.org, Michael Chan
In-Reply-To: <20111215233712.GA7371@mcarlson.broadcom.com>

On Thu, 2011-12-15 at 15:37 -0800, Matt Carlson wrote:
> On Thu, Dec 15, 2011 at 01:13:47PM -0800, Ben Hutchings wrote:
> > On Thu, 2011-12-15 at 13:03 -0800, Matt Carlson wrote:
> > > On Wed, Dec 14, 2011 at 01:50:59PM -0800, Ben Hutchings wrote:
> > > > On Wed, 2011-12-14 at 13:10 -0800, Matt Carlson wrote:
> > [...]
> > > > > +	if (!indir->size) {
> > > > > +		indir->size = TG3_RSS_INDIR_TBL_SIZE;
> > > > > +		return 0;
> > > > > +	}
> > > > > +
> > > > > +	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> > > > > +		return -EINVAL;
> > > > 
> > > > This is enough to make the ethtool command work, but you're really
> > > > supposed to copy min(indir->size, TG3_RSS_INDIR_TBL_SIZE) entries.
> > > 
> > > Could you elaborate on this?  I'm confused because I can't figure out
> > > how returning half of an indirection table could be useful.
> > 
> > It's a generalisation of the zero-length and full-length cases.  But no,
> > it isn't very useful, nor did I actually specify that anywhere!
> > 
> > Maybe there should be a driver operation to get the table size, and then
> > the core can make sure that drivers only ever deal with full-table
> > buffers.  Though that wouldn't cover your reset-to-default case.
> 
> That's how your current implementation of ethtool works, doesn't it?  It
> first makes a call to get_rxfh_indir() with a zero size parameter.  The
> driver interprets this as a query for the size of the driver's
> indirection table.  Subsequent calls to get_rxfh_indir() with a non-zero
> size are interpreted as a request for the indirection table data.

Yes, and I think it's fine if we restrict input that way.

> Right now, I coded get_rxfh_indir() and set_rxfh_indir() to be strict in
> what they accept.  The restrictions can be relaxed though if we can
> outline what the driver should do with partial tables.  (I wonder if
> these types of interpretations are better placed above the driver
> though.

I agree; stand by for patches.

> Then again, maybe not.  Tg3 devices can tune the size of the
> indirection table by powers of two if needed at the cost of a reset.)

You can achieve the same effect repeating the entries of the smaller
table multiple times.

> But IMO all these ideas make the reset-to-default case more desirable.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
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 net-next 1/3] ethtool: Clarify use of size field for ETHTOOL_GRXFHINDIR
From: Ben Hutchings @ 2011-12-15 23:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, Matt Carlson

In order to find out the device's RX flow hash table size, ethtool
initially uses ETHTOOL_GRXFHINDIR with a buffer size of zero.  This
must be supported, but it is not necessary to support any other user
buffer size less than the device table size.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 include/linux/ethtool.h |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 20db5b27..0ec2fd4 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -543,8 +543,9 @@ struct compat_ethtool_rxnfc {
 /**
  * struct ethtool_rxfh_indir - command to get or set RX flow hash indirection
  * @cmd: Specific command number - %ETHTOOL_GRXFHINDIR or %ETHTOOL_SRXFHINDIR
- * @size: On entry, the array size of the user buffer.  On return from
- *	%ETHTOOL_GRXFHINDIR, the array size of the hardware indirection table.
+ * @size: On entry, the array size of the user buffer, which may be zero
+ *	for %ETHTOOL_GRXFHINDIR.  On return from %ETHTOOL_GRXFHINDIR, the
+ *	array size of the hardware indirection table.
  * @ring_index: RX ring/queue index for each hash value
  */
 struct ethtool_rxfh_indir {
-- 
1.7.4.4



-- 
Ben Hutchings, Staff Engineer, Solarflare
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 related

* [PATCH net-next 2/3] ethtool: Centralise validation of ETHTOOL_{G,S}RXFHINDIR parameters
From: Ben Hutchings @ 2011-12-15 23:55 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-net-drivers, Matt Carlson, Eilon Greenstein,
	Dimitris Michailidis, Shreyas Bhatewara, VMware, Inc.
In-Reply-To: <1323993076.2773.23.camel@bwh-desktop>

Add a new ethtool operation (get_rxfh_indir_size) to get the
indirectional table size.  Use this to validate the user buffer size
before calling get_rxfh_indir or set_rxfh_indir.  Use get_rxnfc to get
the number of RX rings, and validate the contents of the new
indirection table before calling set_rxfh_indir.  Remove this
validation from drivers.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
The bnx2x, cxgb4 and vmxnet3 changes are compile-tested only.

Ben.

 .../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c    |   39 ++++------
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c    |   27 ++++---
 drivers/net/ethernet/sfc/ethtool.c                 |   35 +++------
 drivers/net/vmxnet3/vmxnet3_ethtool.c              |   35 ++++-----
 include/linux/ethtool.h                            |   11 ++-
 net/core/ethtool.c                                 |   81 +++++++++++++------
 6 files changed, 117 insertions(+), 111 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 90d44af..a688b9d 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -2302,18 +2302,20 @@ static int bnx2x_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
 	}
 }
 
-static int bnx2x_get_rxfh_indir(struct net_device *dev,
-				struct ethtool_rxfh_indir *indir)
+static u32 bnx2x_get_rxfh_indir_size(struct net_device *dev)
+{
+	struct bnx2x *bp = netdev_priv(dev);
+
+	return (bp->multi_mode == ETH_RSS_MODE_DISABLED ?
+		0 : T_ETH_INDIRECTION_TABLE_SIZE);
+}
+
+static int bnx2x_get_rxfh_indir(struct net_device *dev, u32 *indir)
 {
 	struct bnx2x *bp = netdev_priv(dev);
-	size_t copy_size =
-		min_t(size_t, indir->size, T_ETH_INDIRECTION_TABLE_SIZE);
 	u8 ind_table[T_ETH_INDIRECTION_TABLE_SIZE] = {0};
 	size_t i;
 
-	if (bp->multi_mode == ETH_RSS_MODE_DISABLED)
-		return -EOPNOTSUPP;
-
 	/* Get the current configuration of the RSS indirection table */
 	bnx2x_get_rss_ind_table(&bp->rss_conf_obj, ind_table);
 
@@ -2326,33 +2328,19 @@ static int bnx2x_get_rxfh_indir(struct net_device *dev,
 	 * align the returned table to the Client ID of the leading RSS
 	 * queue.
 	 */
-	for (i = 0; i < copy_size; i++)
-		indir->ring_index[i] = ind_table[i] - bp->fp->cl_id;
-
-	indir->size = T_ETH_INDIRECTION_TABLE_SIZE;
+	for (i = 0; i < T_ETH_INDIRECTION_TABLE_SIZE; i++)
+		indir[i] = ind_table[i] - bp->fp->cl_id;
 
 	return 0;
 }
 
-static int bnx2x_set_rxfh_indir(struct net_device *dev,
-				const struct ethtool_rxfh_indir *indir)
+static int bnx2x_set_rxfh_indir(struct net_device *dev, const u32 *indir)
 {
 	struct bnx2x *bp = netdev_priv(dev);
 	size_t i;
 	u8 ind_table[T_ETH_INDIRECTION_TABLE_SIZE] = {0};
-	u32 num_eth_queues = BNX2X_NUM_ETH_QUEUES(bp);
-
-	if (bp->multi_mode == ETH_RSS_MODE_DISABLED)
-		return -EOPNOTSUPP;
-
-	/* validate the size */
-	if (indir->size != T_ETH_INDIRECTION_TABLE_SIZE)
-		return -EINVAL;
 
 	for (i = 0; i < T_ETH_INDIRECTION_TABLE_SIZE; i++) {
-		/* validate the indices */
-		if (indir->ring_index[i] >= num_eth_queues)
-			return -EINVAL;
 		/*
 		 * The same as in bnx2x_get_rxfh_indir: we can't use a memcpy()
 		 * as an internal storage of an indirection table is a u8 array
@@ -2362,7 +2350,7 @@ static int bnx2x_set_rxfh_indir(struct net_device *dev,
 		 * align the received table to the Client ID of the leading RSS
 		 * queue
 		 */
-		ind_table[i] = indir->ring_index[i] + bp->fp->cl_id;
+		ind_table[i] = indir[i] + bp->fp->cl_id;
 	}
 
 	return bnx2x_config_rss_pf(bp, ind_table, false);
@@ -2395,6 +2383,7 @@ static const struct ethtool_ops bnx2x_ethtool_ops = {
 	.set_phys_id		= bnx2x_set_phys_id,
 	.get_ethtool_stats	= bnx2x_get_ethtool_stats,
 	.get_rxnfc		= bnx2x_get_rxnfc,
+	.get_rxfh_indir_size	= bnx2x_get_rxfh_indir_size,
 	.get_rxfh_indir		= bnx2x_get_rxfh_indir,
 	.set_rxfh_indir		= bnx2x_set_rxfh_indir,
 };
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index a34e7ce..8ffd55b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -1871,30 +1871,30 @@ static int cxgb_set_features(struct net_device *dev, netdev_features_t features)
 	return err;
 }
 
-static int get_rss_table(struct net_device *dev, struct ethtool_rxfh_indir *p)
+static u32 get_rss_table_size(struct net_device *dev)
 {
 	const struct port_info *pi = netdev_priv(dev);
-	unsigned int n = min_t(unsigned int, p->size, pi->rss_size);
 
-	p->size = pi->rss_size;
+	return pi->rss_size;
+}
+
+static int get_rss_table(struct net_device *dev, u32 *p)
+{
+	const struct port_info *pi = netdev_priv(dev);
+	unsigned int n = pi->rss_size;
+
 	while (n--)
-		p->ring_index[n] = pi->rss[n];
+		p[n] = pi->rss[n];
 	return 0;
 }
 
-static int set_rss_table(struct net_device *dev,
-			 const struct ethtool_rxfh_indir *p)
+static int set_rss_table(struct net_device *dev, const u32 *p)
 {
 	unsigned int i;
 	struct port_info *pi = netdev_priv(dev);
 
-	if (p->size != pi->rss_size)
-		return -EINVAL;
-	for (i = 0; i < p->size; i++)
-		if (p->ring_index[i] >= pi->nqsets)
-			return -EINVAL;
-	for (i = 0; i < p->size; i++)
-		pi->rss[i] = p->ring_index[i];
+	for (i = 0; i < pi->rss_size; i++)
+		pi->rss[i] = p[i];
 	if (pi->adapter->flags & FULL_INIT_DONE)
 		return write_rss(pi, pi->rss);
 	return 0;
@@ -1989,6 +1989,7 @@ static struct ethtool_ops cxgb_ethtool_ops = {
 	.get_wol           = get_wol,
 	.set_wol           = set_wol,
 	.get_rxnfc         = get_rxnfc,
+	.get_rxfh_indir_size = get_rss_table_size,
 	.get_rxfh_indir    = get_rss_table,
 	.set_rxfh_indir    = set_rss_table,
 	.flash_device      = set_flash,
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index f3cd96d..1be51b2 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -956,40 +956,28 @@ static int efx_ethtool_set_rx_ntuple(struct net_device *net_dev,
 	return rc < 0 ? rc : 0;
 }
 
-static int efx_ethtool_get_rxfh_indir(struct net_device *net_dev,
-				      struct ethtool_rxfh_indir *indir)
+static u32 efx_ethtool_get_rxfh_indir_size(struct net_device *net_dev)
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
-	size_t copy_size =
-		min_t(size_t, indir->size, ARRAY_SIZE(efx->rx_indir_table));
 
-	if (efx_nic_rev(efx) < EFX_REV_FALCON_B0)
-		return -EOPNOTSUPP;
+	return (efx_nic_rev(efx) < EFX_REV_FALCON_B0 ?
+		0 : ARRAY_SIZE(efx->rx_indir_table));
+}
+
+static int efx_ethtool_get_rxfh_indir(struct net_device *net_dev, u32 *indir)
+{
+	struct efx_nic *efx = netdev_priv(net_dev);
 
-	indir->size = ARRAY_SIZE(efx->rx_indir_table);
-	memcpy(indir->ring_index, efx->rx_indir_table,
-	       copy_size * sizeof(indir->ring_index[0]));
+	memcpy(indir, efx->rx_indir_table, sizeof(efx->rx_indir_table));
 	return 0;
 }
 
 static int efx_ethtool_set_rxfh_indir(struct net_device *net_dev,
-				      const struct ethtool_rxfh_indir *indir)
+				      const u32 *indir)
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
-	size_t i;
-
-	if (efx_nic_rev(efx) < EFX_REV_FALCON_B0)
-		return -EOPNOTSUPP;
-
-	/* Validate size and indices */
-	if (indir->size != ARRAY_SIZE(efx->rx_indir_table))
-		return -EINVAL;
-	for (i = 0; i < ARRAY_SIZE(efx->rx_indir_table); i++)
-		if (indir->ring_index[i] >= efx->n_rx_channels)
-			return -EINVAL;
 
-	memcpy(efx->rx_indir_table, indir->ring_index,
-	       sizeof(efx->rx_indir_table));
+	memcpy(efx->rx_indir_table, indir, sizeof(efx->rx_indir_table));
 	efx_nic_push_rx_indir_table(efx);
 	return 0;
 }
@@ -1020,6 +1008,7 @@ const struct ethtool_ops efx_ethtool_ops = {
 	.reset			= efx_ethtool_reset,
 	.get_rxnfc		= efx_ethtool_get_rxnfc,
 	.set_rx_ntuple		= efx_ethtool_set_rx_ntuple,
+	.get_rxfh_indir_size	= efx_ethtool_get_rxfh_indir_size,
 	.get_rxfh_indir		= efx_ethtool_get_rxfh_indir,
 	.set_rxfh_indir		= efx_ethtool_set_rxfh_indir,
 };
diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index b492ee1..a3eb75a 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -565,44 +565,38 @@ vmxnet3_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *info,
 }
 
 #ifdef VMXNET3_RSS
+static u32
+vmxnet3_get_rss_indir_size(struct net_device *netdev)
+{
+	struct vmxnet3_adapter *adapter = netdev_priv(netdev);
+	struct UPT1_RSSConf *rssConf = adapter->rss_conf;
+
+	return rssConf->indTableSize;
+}
+
 static int
-vmxnet3_get_rss_indir(struct net_device *netdev,
-		      struct ethtool_rxfh_indir *p)
+vmxnet3_get_rss_indir(struct net_device *netdev, u32 *p)
 {
 	struct vmxnet3_adapter *adapter = netdev_priv(netdev);
 	struct UPT1_RSSConf *rssConf = adapter->rss_conf;
-	unsigned int n = min_t(unsigned int, p->size, rssConf->indTableSize);
+	unsigned int n = rssConf->indTableSize;
 
-	p->size = rssConf->indTableSize;
 	while (n--)
-		p->ring_index[n] = rssConf->indTable[n];
+		p[n] = rssConf->indTable[n];
 	return 0;
 
 }
 
 static int
-vmxnet3_set_rss_indir(struct net_device *netdev,
-		      const struct ethtool_rxfh_indir *p)
+vmxnet3_set_rss_indir(struct net_device *netdev, const u32 *p)
 {
 	unsigned int i;
 	unsigned long flags;
 	struct vmxnet3_adapter *adapter = netdev_priv(netdev);
 	struct UPT1_RSSConf *rssConf = adapter->rss_conf;
 
-	if (p->size != rssConf->indTableSize)
-		return -EINVAL;
-	for (i = 0; i < rssConf->indTableSize; i++) {
-		/*
-		 * Return with error code if any of the queue indices
-		 * is out of range
-		 */
-		if (p->ring_index[i] < 0 ||
-		    p->ring_index[i] >= adapter->num_rx_queues)
-			return -EINVAL;
-	}
-
 	for (i = 0; i < rssConf->indTableSize; i++)
-		rssConf->indTable[i] = p->ring_index[i];
+		rssConf->indTable[i] = p[i];
 
 	spin_lock_irqsave(&adapter->cmd_lock, flags);
 	VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
@@ -629,6 +623,7 @@ static struct ethtool_ops vmxnet3_ethtool_ops = {
 	.set_ringparam     = vmxnet3_set_ringparam,
 	.get_rxnfc         = vmxnet3_get_rxnfc,
 #ifdef VMXNET3_RSS
+	.get_rxfh_indir_size = vmxnet3_get_rss_indir_size,
 	.get_rxfh_indir    = vmxnet3_get_rss_indir,
 	.set_rxfh_indir    = vmxnet3_set_rss_indir,
 #endif
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 0ec2fd4..3b9f09d 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -828,9 +828,13 @@ u32 ethtool_op_get_link(struct net_device *dev);
  *	error code or zero.
  * @set_rx_ntuple: Set an RX n-tuple rule.  Returns a negative error code
  *	or zero.
+ * @get_rxfh_indir_size: Get the size of the RX flow hash indirection table.
+ *	Returns zero if not supported for this specific device.
  * @get_rxfh_indir: Get the contents of the RX flow hash indirection table.
+ *	Will not be called if @get_rxfh_indir_size returns zero.
  *	Returns a negative error code or zero.
  * @set_rxfh_indir: Set the contents of the RX flow hash indirection table.
+ *	Will not be called if @get_rxfh_indir_size returns zero.
  *	Returns a negative error code or zero.
  * @get_channels: Get number of channels.
  * @set_channels: Set number of channels.  Returns a negative error code or
@@ -894,10 +898,9 @@ struct ethtool_ops {
 	int	(*reset)(struct net_device *, u32 *);
 	int	(*set_rx_ntuple)(struct net_device *,
 				 struct ethtool_rx_ntuple *);
-	int	(*get_rxfh_indir)(struct net_device *,
-				  struct ethtool_rxfh_indir *);
-	int	(*set_rxfh_indir)(struct net_device *,
-				  const struct ethtool_rxfh_indir *);
+	u32	(*get_rxfh_indir_size)(struct net_device *);
+	int	(*get_rxfh_indir)(struct net_device *, u32 *);
+	int	(*set_rxfh_indir)(struct net_device *, const u32 *);
 	void	(*get_channels)(struct net_device *, struct ethtool_channels *);
 	int	(*set_channels)(struct net_device *, struct ethtool_channels *);
 	int	(*get_dump_flag)(struct net_device *, struct ethtool_dump *);
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 31b0b7f..69f71b8 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -515,34 +515,44 @@ err_out:
 static noinline_for_stack int ethtool_get_rxfh_indir(struct net_device *dev,
 						     void __user *useraddr)
 {
-	struct ethtool_rxfh_indir *indir;
-	u32 table_size;
-	size_t full_size;
+	u32 user_size, dev_size;
+	u32 *indir;
 	int ret;
 
-	if (!dev->ethtool_ops->get_rxfh_indir)
+	if (!dev->ethtool_ops->get_rxfh_indir_size ||
+	    !dev->ethtool_ops->get_rxfh_indir)
+		return -EOPNOTSUPP;
+	dev_size = dev->ethtool_ops->get_rxfh_indir_size(dev);
+	if (dev_size == 0)
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(&table_size,
+	if (copy_from_user(&user_size,
 			   useraddr + offsetof(struct ethtool_rxfh_indir, size),
-			   sizeof(table_size)))
+			   sizeof(user_size)))
 		return -EFAULT;
 
-	if (table_size >
-	    (KMALLOC_MAX_SIZE - sizeof(*indir)) / sizeof(*indir->ring_index))
-		return -ENOMEM;
-	full_size = sizeof(*indir) + sizeof(*indir->ring_index) * table_size;
-	indir = kzalloc(full_size, GFP_USER);
+	if (copy_to_user(useraddr + offsetof(struct ethtool_rxfh_indir, size),
+			 &dev_size, sizeof(dev_size)))
+		return -EFAULT;
+
+	/* If the user buffer size is 0, this is just a query for the
+	 * device table size.  Otherwise, if it's smaller than the
+	 * device table size it's an error.
+	 */
+	if (user_size < dev_size)
+		return user_size == 0 ? 0 : -EINVAL;
+
+	indir = kcalloc(dev_size, sizeof(indir[0]), GFP_USER);
 	if (!indir)
 		return -ENOMEM;
 
-	indir->cmd = ETHTOOL_GRXFHINDIR;
-	indir->size = table_size;
 	ret = dev->ethtool_ops->get_rxfh_indir(dev, indir);
 	if (ret)
 		goto out;
 
-	if (copy_to_user(useraddr, indir, full_size))
+	if (copy_to_user(useraddr +
+			 offsetof(struct ethtool_rxfh_indir, ring_index[0]),
+			 indir, dev_size * sizeof(indir[0])))
 		ret = -EFAULT;
 
 out:
@@ -553,32 +563,51 @@ out:
 static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev,
 						     void __user *useraddr)
 {
-	struct ethtool_rxfh_indir *indir;
-	u32 table_size;
-	size_t full_size;
+	struct ethtool_rxnfc rx_rings;
+	u32 user_size, dev_size, i;
+	u32 *indir;
 	int ret;
 
-	if (!dev->ethtool_ops->set_rxfh_indir)
+	if (!dev->ethtool_ops->get_rxfh_indir_size ||
+	    !dev->ethtool_ops->set_rxfh_indir ||
+	    !dev->ethtool_ops->get_rxnfc)
+		return -EOPNOTSUPP;
+	dev_size = dev->ethtool_ops->get_rxfh_indir_size(dev);
+	if (dev_size == 0)
 		return -EOPNOTSUPP;
 
-	if (copy_from_user(&table_size,
+	if (copy_from_user(&user_size,
 			   useraddr + offsetof(struct ethtool_rxfh_indir, size),
-			   sizeof(table_size)))
+			   sizeof(user_size)))
 		return -EFAULT;
 
-	if (table_size >
-	    (KMALLOC_MAX_SIZE - sizeof(*indir)) / sizeof(*indir->ring_index))
-		return -ENOMEM;
-	full_size = sizeof(*indir) + sizeof(*indir->ring_index) * table_size;
-	indir = kmalloc(full_size, GFP_USER);
+	if (user_size != dev_size)
+		return -EINVAL;
+
+	indir = kcalloc(dev_size, sizeof(indir[0]), GFP_USER);
 	if (!indir)
 		return -ENOMEM;
 
-	if (copy_from_user(indir, useraddr, full_size)) {
+	if (copy_from_user(indir,
+			   useraddr +
+			   offsetof(struct ethtool_rxfh_indir, ring_index[0]),
+			   dev_size * sizeof(indir[0]))) {
 		ret = -EFAULT;
 		goto out;
 	}
 
+	/* Validate ring indices */
+	rx_rings.cmd = ETHTOOL_GRXRINGS;
+	ret = dev->ethtool_ops->get_rxnfc(dev, &rx_rings, NULL);
+	if (ret)
+		goto out;
+	for (i = 0; i < dev_size; i++) {
+		if (indir[i] >= rx_rings.data) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
 	ret = dev->ethtool_ops->set_rxfh_indir(dev, indir);
 
 out:
-- 
1.7.4.4



-- 
Ben Hutchings, Staff Engineer, Solarflare
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 related

* Re: [PATCH 14/15] module_param: make bool parameters really bool (net & drivers/net)
From: Rusty Russell @ 2011-12-15 23:14 UTC (permalink / raw)
  To: Joe Perches
  Cc: lkml - Kernel Mailing List, Pawel Moll, David S. Miller, netdev
In-Reply-To: <1323920752.29500.11.camel@joe2Laptop>

On Wed, 14 Dec 2011 19:45:52 -0800, Joe Perches <joe@perches.com> wrote:
> On Thu, 2011-12-15 at 13:51 +1030, Rusty Russell wrote:
> > module_param(bool) used to counter-intuitively take an int.  In
> > fddd5201 (mid-2009) we allowed bool or int/unsigned int using a messy
> > trick.
> []
> > diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> []
> > @@ -38,15 +38,15 @@ MODULE_ALIAS_LDISC(N_CAIF);
> []
> > -static int ser_use_stx = 1;
> > +static bool ser_use_stx = 1;
> 
> Could you respin these please using bool foo = true/false instead?

I could, but as I was touching 457 files, so I was aiming for minimal
changes.

Dave, did you want a true/false cleanup too?

Thanks,
Rusty.

^ permalink raw reply

* [PATCH net-next 3/3] ethtool: Define and apply a default policy for RX flow hash indirection
From: Ben Hutchings @ 2011-12-15 23:56 UTC (permalink / raw)
  To: David Miller
  Cc: linux-net-drivers, Matt Carlson, Eilon Greenstein,
	Dimitris Michailidis, Shreyas Bhatewara, netdev, VMware, Inc.
In-Reply-To: <1323993076.2773.23.camel@bwh-desktop>

All drivers that support modification of the RX flow hash indirection
table initialise it in the same way: RX rings are assigned to table
entries in rotation.  Make that default policy explicit by having them
call a ethtool_rxfh_indir_default() function.

In the ethtool core, add support for a zero size value for
ETHTOOL_SRXFHINDIR, which resets the table to this default.

Partly-suggested-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
The bnx2x, cxgb4 and vmxnet3 changes are compile-tested only.

Ben.

 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c |    3 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |    2 +-
 drivers/net/ethernet/sfc/efx.c                  |    3 +-
 drivers/net/vmxnet3/vmxnet3_drv.c               |    3 +-
 include/linux/ethtool.h                         |   23 ++++++++++++++--
 net/core/ethtool.c                              |   33 ++++++++++++++---------
 6 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 64f5cf5..2b731b2 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -1545,7 +1545,8 @@ static inline int bnx2x_init_rss_pf(struct bnx2x *bp)
 	if (bp->multi_mode != ETH_RSS_MODE_DISABLED) {
 		for (i = 0; i < sizeof(ind_table); i++)
 			ind_table[i] =
-				bp->fp->cl_id +	(i % num_eth_queues);
+				bp->fp->cl_id +
+				ethtool_rxfh_indir_default(i, num_eth_queues);
 	}
 
 	/*
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 8ffd55b..fccbe49 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -3449,7 +3449,7 @@ static int __devinit init_rss(struct adapter *adap)
 		if (!pi->rss)
 			return -ENOMEM;
 		for (j = 0; j < pi->rss_size; j++)
-			pi->rss[j] = j % pi->nqsets;
+			pi->rss[j] = ethtool_rxfh_indir_default(j, pi->nqsets);
 	}
 	return 0;
 }
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 14e134d..44a82c6 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1336,7 +1336,8 @@ static int efx_probe_nic(struct efx_nic *efx)
 	if (efx->n_channels > 1)
 		get_random_bytes(&efx->rx_hash_key, sizeof(efx->rx_hash_key));
 	for (i = 0; i < ARRAY_SIZE(efx->rx_indir_table); i++)
-		efx->rx_indir_table[i] = i % efx->n_rx_channels;
+		efx->rx_indir_table[i] =
+			ethtool_rxfh_indir_default(i, efx->n_rx_channels);
 
 	efx_set_channels(efx);
 	netif_set_real_num_tx_queues(efx->net_dev, efx->n_tx_channels);
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 1c2ae11..de7fc34 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2167,7 +2167,8 @@ vmxnet3_setup_driver_shared(struct vmxnet3_adapter *adapter)
 		rssConf->indTableSize = VMXNET3_RSS_IND_TABLE_SIZE;
 		get_random_bytes(&rssConf->hashKey[0], rssConf->hashKeySize);
 		for (i = 0; i < rssConf->indTableSize; i++)
-			rssConf->indTable[i] = i % adapter->num_rx_queues;
+			rssConf->indTable[i] = ethtool_rxfh_indir_default(
+				i, adapter->num_rx_queues);
 
 		devRead->rssConfDesc.confVer = 1;
 		devRead->rssConfDesc.confLen = sizeof(*rssConf);
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3b9f09d..7eba8f3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -543,10 +543,15 @@ struct compat_ethtool_rxnfc {
 /**
  * struct ethtool_rxfh_indir - command to get or set RX flow hash indirection
  * @cmd: Specific command number - %ETHTOOL_GRXFHINDIR or %ETHTOOL_SRXFHINDIR
- * @size: On entry, the array size of the user buffer, which may be zero
- *	for %ETHTOOL_GRXFHINDIR.  On return from %ETHTOOL_GRXFHINDIR, the
- *	array size of the hardware indirection table.
+ * @size: On entry, the array size of the user buffer, which may be zero.
+ *	On return from %ETHTOOL_GRXFHINDIR, the array size of the hardware
+ *	indirection table. 
  * @ring_index: RX ring/queue index for each hash value
+ *
+ * For %ETHTOOL_GRXFHINDIR, a @size of zero means that only the size
+ * should be returned.  For %ETHTOOL_SRXFHINDIR, a @size of zero means
+ * the table should be reset to default values.  This last feature
+ * is not supported by the original implementations.
  */
 struct ethtool_rxfh_indir {
 	__u32	cmd;
@@ -750,6 +755,18 @@ struct net_device;
 u32 ethtool_op_get_link(struct net_device *dev);
 
 /**
+ * ethtool_rxfh_indir_default - get default value for RX flow hash indirection
+ * @index: Index in RX flow hash indirection table
+ * @n_rx_rings: Number of RX rings to use
+ *
+ * This function provides the default policy for RX flow hash indirection.
+ */
+static inline u32 ethtool_rxfh_indir_default(u32 index, u32 n_rx_rings)
+{
+	return index % n_rx_rings;
+}
+
+/**
  * struct ethtool_ops - optional netdev operations
  * @get_settings: Get various device settings including Ethernet link
  *	settings. The @cmd parameter is expected to have been cleared
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 69f71b8..597732c 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -581,31 +581,38 @@ static noinline_for_stack int ethtool_set_rxfh_indir(struct net_device *dev,
 			   sizeof(user_size)))
 		return -EFAULT;
 
-	if (user_size != dev_size)
+	if (user_size != 0 && user_size != dev_size)
 		return -EINVAL;
 
 	indir = kcalloc(dev_size, sizeof(indir[0]), GFP_USER);
 	if (!indir)
 		return -ENOMEM;
 
-	if (copy_from_user(indir,
-			   useraddr +
-			   offsetof(struct ethtool_rxfh_indir, ring_index[0]),
-			   dev_size * sizeof(indir[0]))) {
-		ret = -EFAULT;
-		goto out;
-	}
-
-	/* Validate ring indices */
 	rx_rings.cmd = ETHTOOL_GRXRINGS;
 	ret = dev->ethtool_ops->get_rxnfc(dev, &rx_rings, NULL);
 	if (ret)
 		goto out;
-	for (i = 0; i < dev_size; i++) {
-		if (indir[i] >= rx_rings.data) {
-			ret = -EINVAL;
+
+	if (user_size == 0) {
+		for (i = 0; i < dev_size; i++)
+			indir[i] = ethtool_rxfh_indir_default(i, rx_rings.data);
+	} else {
+		if (copy_from_user(indir,
+				  useraddr +
+				  offsetof(struct ethtool_rxfh_indir,
+					   ring_index[0]),
+				  dev_size * sizeof(indir[0]))) {
+			ret = -EFAULT;
 			goto out;
 		}
+
+		/* Validate ring indices */
+		for (i = 0; i < dev_size; i++) {
+			if (indir[i] >= rx_rings.data) {
+				ret = -EINVAL;
+				goto out;
+			}
+		}
 	}
 
 	ret = dev->ethtool_ops->set_rxfh_indir(dev, indir);
-- 
1.7.4.4


-- 
Ben Hutchings, Staff Engineer, Solarflare
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 related

* Re: [PATCH 14/15] module_param: make bool parameters really bool (net & drivers/net)
From: Joe Perches @ 2011-12-16  0:33 UTC (permalink / raw)
  To: Rusty Russell
  Cc: lkml - Kernel Mailing List, Pawel Moll, David S. Miller, netdev
In-Reply-To: <87k45xqvfs.fsf@rustcorp.com.au>

On Fri, 2011-12-16 at 09:44 +1030, Rusty Russell wrote:
> I could, but as I was touching 457 files, so I was aiming for minimal
> changes.

> Dave, did you want a true/false cleanup too?

Hi Rusty.

Did you perform these transforms via spatch?
If not, here's a trivial spatch script for those files.

$ cat bool.cocci
@@
bool b
@@
-b = 0
+b = false
@@
bool b
@@
-b = 1
+b = true

^ permalink raw reply

* [PATCH] tg3: Make the RSS indir tbl admin configurable
From: Matt Carlson @ 2011-12-16  0:47 UTC (permalink / raw)
  To: davem; +Cc: netdev, mcarlson, Michael Chan, Ben Hutchings

This patch adds the ethtool callbacks necessary to change the rss
indirection table from userspace.  When setting the indirection table
through set_rxfh_indir, an indirection table size of zero is
interpreted to mean that the admin wants to relinquish control of the
table to the driver.  Should the number of interrupts change (e.g.
across a close / open call, or through a reset), any indirection table
values that exceed the number of RSS queues or interrupt vectors will
be automatically scaled back to values within range.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Reviewed-by: Benjamin Li <benli@broadcom.com>
---
 drivers/net/ethernet/broadcom/tg3.c |  126 ++++++++++++++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/tg3.h |    1 +
 2 files changed, 126 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 8bf11ca..87f70f6 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -8229,9 +8229,19 @@ void tg3_rss_init_indir_tbl(struct tg3 *tp)
 
 	if (tp->irq_cnt <= 2)
 		memset(&tp->rss_ind_tbl[0], 0, sizeof(tp->rss_ind_tbl));
-	else
+	else if (tg3_flag(tp, USER_INDIR_TBL)) {
+		/* Validate table against current IRQ count */
+		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++) {
+			if (tp->rss_ind_tbl[i] >= tp->irq_cnt - 1) {
+				/* Bring the index within range */
+				tp->rss_ind_tbl[i] = tp->rss_ind_tbl[i] %
+						     (tp->irq_cnt - 1);
+			}
+		}
+	} else {
 		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
 			tp->rss_ind_tbl[i] = i % (tp->irq_cnt - 1);
+	}
 }
 
 void tg3_rss_write_indir_tbl(struct tg3 *tp)
@@ -10719,6 +10729,117 @@ static int tg3_get_sset_count(struct net_device *dev, int sset)
 	}
 }
 
+static int tg3_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
+			 u32 *rules __always_unused)
+{
+	struct tg3 *tp = netdev_priv(dev);
+
+	if (!tg3_flag(tp, SUPPORT_MSIX))
+		return -EOPNOTSUPP;
+
+	switch (info->cmd) {
+	case ETHTOOL_GRXRINGS:
+		if (netif_running(tp->dev))
+			info->data = tp->irq_cnt;
+		else {
+			info->data = num_online_cpus();
+			if (info->data > TG3_IRQ_MAX_VECS_RSS)
+				info->data = TG3_IRQ_MAX_VECS_RSS;
+		}
+
+		/* The first interrupt vector only
+		 * handles link interrupts.
+		 */
+		info->data -= 1;
+		return 0;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int tg3_get_rxfh_indir(struct net_device *dev,
+			      struct ethtool_rxfh_indir *indir)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	int i;
+
+	if (!tg3_flag(tp, SUPPORT_MSIX))
+		return -EOPNOTSUPP;
+
+	if (!indir->size) {
+		indir->size = TG3_RSS_INDIR_TBL_SIZE;
+		return 0;
+	}
+
+	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
+		return -EINVAL;
+
+	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+		indir->ring_index[i] = tp->rss_ind_tbl[i];
+
+	return 0;
+}
+
+static int tg3_set_rxfh_indir(struct net_device *dev,
+			      const struct ethtool_rxfh_indir *indir)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	size_t i;
+
+	if (!tg3_flag(tp, SUPPORT_MSIX))
+		return -EOPNOTSUPP;
+
+	if (!indir->size) {
+		tg3_flag_clear(tp, USER_INDIR_TBL);
+		tg3_rss_init_indir_tbl(tp);
+	} else {
+		int limit;
+
+		/* Validate size and indices */
+		if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
+			return -EINVAL;
+
+		if (netif_running(dev))
+			limit = tp->irq_cnt;
+		else {
+			limit = num_online_cpus();
+			if (limit > TG3_IRQ_MAX_VECS_RSS)
+				limit = TG3_IRQ_MAX_VECS_RSS;
+		}
+
+		/* The first interrupt vector only
+		 * handles link interrupts.
+		 */
+		limit -= 1;
+
+		/* Check the indices in the table.
+		 * Leave the existing table unmodified
+		 * if an error is detected.
+		 */
+		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+			if (indir->ring_index[i] >= limit)
+				return -EINVAL;
+
+		tg3_flag_set(tp, USER_INDIR_TBL);
+
+		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
+			tp->rss_ind_tbl[i] = indir->ring_index[i];
+	}
+
+	if (!netif_running(dev) || !tg3_flag(tp, ENABLE_RSS))
+		return 0;
+
+	/* It is legal to write the indirection
+	 * table while the device is running.
+	 */
+	tg3_full_lock(tp, 0);
+	tg3_rss_write_indir_tbl(tp);
+	tg3_full_unlock(tp);
+
+	return 0;
+}
+
 static void tg3_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 {
 	switch (stringset) {
@@ -11949,6 +12070,9 @@ static const struct ethtool_ops tg3_ethtool_ops = {
 	.get_coalesce		= tg3_get_coalesce,
 	.set_coalesce		= tg3_set_coalesce,
 	.get_sset_count		= tg3_get_sset_count,
+	.get_rxnfc		= tg3_get_rxnfc,
+	.get_rxfh_indir		= tg3_get_rxfh_indir,
+	.set_rxfh_indir		= tg3_set_rxfh_indir,
 };
 
 static void __devinit tg3_get_eeprom_size(struct tg3 *tp)
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index aea8f72..4800595 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2932,6 +2932,7 @@ enum TG3_FLAGS {
 	TG3_FLAG_APE_HAS_NCSI,
 	TG3_FLAG_4K_FIFO_LIMIT,
 	TG3_FLAG_RESET_TASK_PENDING,
+	TG3_FLAG_USER_INDIR_TBL,
 	TG3_FLAG_5705_PLUS,
 	TG3_FLAG_IS_5788,
 	TG3_FLAG_5750_PLUS,
-- 
1.7.3.4

^ permalink raw reply related

* Re: [PATCH net-next 2/3] ethtool: Centralise validation of ETHTOOL_{G,S}RXFHINDIR parameters
From: Dimitris Michailidis @ 2011-12-16  0:51 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Miller, netdev, linux-net-drivers, Matt Carlson,
	Eilon Greenstein, Shreyas Bhatewara, VMware, Inc.
In-Reply-To: <1323993301.2773.26.camel@bwh-desktop>

On 12/15/2011 03:55 PM, Ben Hutchings wrote:
> Add a new ethtool operation (get_rxfh_indir_size) to get the
> indirectional table size.  Use this to validate the user buffer size
> before calling get_rxfh_indir or set_rxfh_indir.  Use get_rxnfc to get
> the number of RX rings, and validate the contents of the new
> indirection table before calling set_rxfh_indir.  Remove this
> validation from drivers.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> The bnx2x, cxgb4 and vmxnet3 changes are compile-tested only.

The cxgb4 and core changes look good.

Acked-by: Dimitris Michailidis <dm@chelsio.com>

^ permalink raw reply

* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2011-12-16  1:07 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: linux-next, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

Hi all,

Today's linux-next merge of the net-next tree got a conflict in
net/ipv6/route.c between commit bb3c36863e80 ("ipv6: Check dest prefix
length on original route not copied one in rt6_alloc_cow()") from the net
tree and commit 3830847396fa ("ipv6: Various cleanups in route.c") from
the net-next tree.

Just context changes.  I fixed it up (see below) and can carry the fix as
necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc net/ipv6/route.c
index b582a0a,4bf362b..0000000
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@@ -727,11 -727,11 +727,11 @@@ static struct rt6_info *rt6_alloc_cow(c
  		struct neighbour *neigh;
  		int attempts = !in_softirq();
  
- 		if (!(rt->rt6i_flags&RTF_GATEWAY)) {
+ 		if (!(rt->rt6i_flags & RTF_GATEWAY)) {
 -			if (rt->rt6i_dst.plen != 128 &&
 +			if (ort->rt6i_dst.plen != 128 &&
  			    ipv6_addr_equal(&ort->rt6i_dst.addr, daddr))
  				rt->rt6i_flags |= RTF_ANYCAST;
- 			ipv6_addr_copy(&rt->rt6i_gateway, daddr);
+ 			rt->rt6i_gateway = *daddr;
  		}
  
  		rt->rt6i_flags |= RTF_CACHE;

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 1/3] ethtool: Clarify use of size field for ETHTOOL_GRXFHINDIR
From: Matt Carlson @ 2011-12-16  1:14 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: David Miller, netdev@vger.kernel.org,
	linux-net-drivers@solarflare.com, Matthew Carlson
In-Reply-To: <1323993076.2773.23.camel@bwh-desktop>

On Thu, Dec 15, 2011 at 03:51:16PM -0800, Ben Hutchings wrote:
> In order to find out the device's RX flow hash table size, ethtool
> initially uses ETHTOOL_GRXFHINDIR with a buffer size of zero.  This
> must be supported, but it is not necessary to support any other user
> buffer size less than the device table size.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
>  include/linux/ethtool.h |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 20db5b27..0ec2fd4 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -543,8 +543,9 @@ struct compat_ethtool_rxnfc {
>  /**
>   * struct ethtool_rxfh_indir - command to get or set RX flow hash indirection
>   * @cmd: Specific command number - %ETHTOOL_GRXFHINDIR or %ETHTOOL_SRXFHINDIR
> - * @size: On entry, the array size of the user buffer.  On return from
> - *	%ETHTOOL_GRXFHINDIR, the array size of the hardware indirection table.
> + * @size: On entry, the array size of the user buffer, which may be zero
> + *	for %ETHTOOL_GRXFHINDIR.  On return from %ETHTOOL_GRXFHINDIR, the
> + *	array size of the hardware indirection table.

Can we roll in the documentation request you gave me earlier?  Something
to the effect of:

"A zero size with %ETHTOOL_SRXFHINDIR means userspace relinquishes
control of the indirection table."

>   * @ring_index: RX ring/queue index for each hash value
>   */
>  struct ethtool_rxfh_indir {
> -- 
> 1.7.4.4
> 
> 
> 
> -- 
> Ben Hutchings, Staff Engineer, Solarflare
> 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 2/2] Explicitly call tcp creation and init from memcontrol.c
From: Glauber Costa @ 2011-12-16  2:06 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: davem, linux-kernel, netdev, cgroups, Eric Dumazet,
	Stephen Rothwell
In-Reply-To: <20111216072050.4f49ac66.kamezawa.hiroyu@jp.fujitsu.com>

On 12/16/2011 02:20 AM, KAMEZAWA Hiroyuki wrote:
> On Fri, 16 Dec 2011 01:13:16 +0900
> KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>  wrote:
>
>> On Thu, 15 Dec 2011 13:34:32 +0400
>> Glauber Costa<glommer@parallels.com>  wrote:
>>
>>> Walking the proto_list holds a read_lock, which prevents us from doing
>>> allocations. Splitting the tcp create function into create + init is
>>> good, but it is not enough since create_files will do allocations as well
>>> (dentry ones, mostly).
>>>
>>> Since this does not involve any protocol state, I propose we call the tcp
>>> functions explicitly from memcontrol.c
>>>
>>> With this, we lose by now the ability of doing cgroup memcontrol for
>>> protocols that are loaded as modules. But at least the ones I have in mind
>>> won't really need it (tcp_ipv6 being the only one, but it uses the same data
>>> structures as tcp_ipv4). So I believe this to be the simpler solution to this
>>> problem.
>>>
>>> Signed-off-by: Glauber Costa<glommer@parallels.com>
>>> CC: Hiroyouki Kamezawa<kamezawa.hiroyu@jp.fujitsu.com>
>>> CC: David S. Miller<davem@davemloft.net>
>>> CC: Eric Dumazet<eric.dumazet@gmail.com>
>>> CC: Stephen Rothwell<sfr@canb.auug.org.au>
>>
>> Could you remake the patch onto the 'latest' linux-next ?
>> As Dave mentioned, some bandaids are already applied and this patch hunks.
>
> Applied patches by hand and did small test for hours.
> seems good.
>
> Reviewed-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com>
>
Kame,

Thanks. But see Dave's answer to this: He'd like me to follow a slightly 
different approach (I've attached a patch earlier)

^ permalink raw reply

* Re: [PATCH 1/2] e100: power down PHY if WOL is not enabled
From: Jiang Wang @ 2011-12-16  2:28 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Francis St. Amant, Panchamukhi, Carolyn, miles.ito@intel.com,
	e1000-devel@lists.sourceforge.net, Bruce Allan, Jesse Brandeburg,
	linux-kernel@vger.kernel.org, John Ronciak, Prasanna,
	netdev@vger.kernel.org, Chaitanya Lala, Peter
In-Reply-To: <1323900468.2753.22.camel@bwh-desktop>

Modified the patch.

>From 16c145e8c6b9fdaf94dfdc91211faba88359aaab Mon Sep 17 00:00:00 2001
From: Jiang Wang <Jiang.Wang@riverbed.com>
Date: Thu, 15 Dec 2011 17:08:32 -0800
Subject: [PATCH 1/2] e100: power down PHY if WOL is not enabled

Since the interface will not be used after being put down and WOL is disabled,
just power it off.
When bring up the interface, power on the PHY.

Signed-off-by: Jiang Wang <Jiang.Wang@riverbed.com>
---
 drivers/net/ethernet/intel/e100.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 5a2fdf7..ca994da 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -1449,6 +1449,11 @@ static int e100_phy_init(struct nic *nic)
 		netif_printk(nic, hw, KERN_DEBUG, nic->netdev,
 			     "phy_addr = %d\n", nic->mii.phy_id);
 
+	/* Make sure power to the PHY is enabled */
+	phy_data = mdio_read(nic->netdev, nic->mii.phy_id, MII_BMCR);
+	phy_data &= ~BMCR_PDOWN;
+	mdio_write(nic->netdev, nic->mii.phy_id, MII_BMCR, phy_data);
+
 	/* Get phy ID */
 	id_lo = mdio_read(netdev, nic->mii.phy_id, MII_PHYSID1);
 	id_hi = mdio_read(netdev, nic->mii.phy_id, MII_PHYSID2);
@@ -2261,6 +2266,15 @@ static void e100_down(struct nic *nic)
 	napi_disable(&nic->napi);
 	netif_stop_queue(nic->netdev);
 	e100_hw_reset(nic);
+
+	/* If wake on LAN is not enabled, power down the PHY */
+	if (!(nic->flags & wol_magic)) {
+		uint16_t phy_data;
+		phy_data = mdio_read(nic->netdev, nic->mii.phy_id, MII_BMCR);
+		phy_data |= BMCR_PDOWN;
+		mdio_write(nic->netdev, nic->mii.phy_id, MII_BMCR, phy_data);
+	}
+
 	free_irq(nic->pdev->irq, nic->netdev);
 	del_timer_sync(&nic->watchdog);
 	netif_carrier_off(nic->netdev);
-- 
1.7.1


-------------------------------------
Jiang Wang
Member of Technical Staff
Riverbed Technology
Tel: (408) 522-5109
Email: Jiang.Wang@riverbed.com
www.riverbed.com


-----Original Message-----
From: Ben Hutchings [mailto:bhutchings@solarflare.com] 
Sent: Wednesday, December 14, 2011 2:08 PM
To: Jiang Wang
Cc: Jeff Kirsher; Jesse Brandeburg; Bruce Allan; Carolyn Wyborny; Don Skidmore; Greg Rose; Peter P Waskiewicz Jr; Alex Duyck; John Ronciak; e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Prasanna Panchamukhi; Chaitanya Lala; Francis St. Amant; miles.ito@intel.com
Subject: RE: [PATCH 1/2] e100: power down PHY if WOL is not enabled

On Wed, 2011-12-14 at 21:59 +0000, Jiang Wang wrote:
> Hi Ben,
> 
> Do you mean the WOL is enabled but PHY is powered off somehow? That 
> scenario should never happen. If WOL is enabled, there is no code to 
> power off PHY.
[...]

Initial conditions: interface up, WOL off, PHY on.

1. Bring interface down. PHY is turned off.
2. Turn WOL on.  PHY is still off, but needs to be on.

Ben.

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

------------------------------------------------------------------------------
Learn Windows Azure Live!  Tuesday, Dec 13, 2011
Microsoft is holding a special Learn Windows Azure training event for 
developers. It will provide a great way to learn Windows Azure and what it 
provides. You can attend the event by watching it streamed LIVE online.  
Learn more at http://p.sf.net/sfu/ms-windowsazure
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* Re: [PATCH 2/2] e100: power off PHY after reset when interface is down
From: Jiang Wang @ 2011-12-16  2:29 UTC (permalink / raw)
  To: Jiang Wang, Jeff Kirsher, Jesse Brandeburg, Bruce Allan,
	Carolyn Wyborny, Don Skidmore, Greg Rose, Peter P Waskiewicz Jr,
	Alex Duyck, John Ronciak, e1000-devel@lists.sourceforge.net,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: Chaitanya Lala, Francis St. Amant, Prasanna Panchamukhi,
	miles.ito@intel.com
In-Reply-To: <1323830996-16388-2-git-send-email-Jiang.Wang@riverbed.com>

Modified the patch.

>From a74d3114934a82d17a43eae7b5bb1488d2c1b6c8 Mon Sep 17 00:00:00 2001
From: Jiang Wang <Jiang.Wang@riverbed.com>
Date: Thu, 15 Dec 2011 17:44:20 -0800
Subject: [PATCH 2/2] e100: power off PHY after reset when interface is down

PHYs supported by e100 re-starts auto-negotiation after writing to
BMCR_RESET bit. This patch powers down PHY when the interface is down
and reset is issued.

Signed-off-by: Jiang Wang <Jiang.Wang@riverbed.com>
---
 drivers/net/ethernet/intel/e100.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index ca994da..2764cdd 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -589,6 +589,7 @@ struct nic {
 		multicast_all      = (1 << 2),
 		wol_magic          = (1 << 3),
 		ich_10h_workaround = (1 << 4),
+		powered_off        = (1 << 5),
 	} flags					____cacheline_aligned;
 
 	enum mac mac;
@@ -1454,6 +1455,8 @@ static int e100_phy_init(struct nic *nic)
 	phy_data &= ~BMCR_PDOWN;
 	mdio_write(nic->netdev, nic->mii.phy_id, MII_BMCR, phy_data);
 
+	nic->flags &= ~powered_off;
+
 	/* Get phy ID */
 	id_lo = mdio_read(netdev, nic->mii.phy_id, MII_PHYSID1);
 	id_hi = mdio_read(netdev, nic->mii.phy_id, MII_PHYSID2);
@@ -2273,6 +2276,7 @@ static void e100_down(struct nic *nic)
 		phy_data = mdio_read(nic->netdev, nic->mii.phy_id, MII_BMCR);
 		phy_data |= BMCR_PDOWN;
 		mdio_write(nic->netdev, nic->mii.phy_id, MII_BMCR, phy_data);
+		nic->flags |= powered_off;
 	}
 
 	free_irq(nic->pdev->irq, nic->netdev);
@@ -2383,6 +2387,13 @@ static int e100_set_settings(struct net_device *netdev, struct ethtool_cmd *cmd)
 	err = mii_ethtool_sset(&nic->mii, cmd);
 	e100_exec_cb(nic, NULL, e100_configure);
 
+	if (nic->flags & powered_off) {
+		uint16_t phy_data;
+		phy_data = mdio_read(nic->netdev, nic->mii.phy_id, MII_BMCR);
+		phy_data |= BMCR_PDOWN;
+		mdio_write(nic->netdev, nic->mii.phy_id, MII_BMCR, phy_data);
+	}
+
 	return err;
 }
 
-- 
1.7.1


-------------------------------------
Jiang Wang
Member of Technical Staff
Riverbed Technology
Tel: (408) 522-5109
Email: Jiang.Wang@riverbed.com
www.riverbed.com


-----Original Message-----
From: Jiang Wang [mailto:Jiang.Wang@riverbed.com] 
Sent: Tuesday, December 13, 2011 6:50 PM
To: Jeff Kirsher; Jesse Brandeburg; Bruce Allan; Carolyn Wyborny; Don Skidmore; Greg Rose; Peter P Waskiewicz Jr; Alex Duyck; John Ronciak; e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Cc: Prasanna Panchamukhi; Chaitanya Lala; Francis St. Amant; miles.ito@intel.com; Jiang Wang
Subject: [PATCH 2/2] e100: power off PHY after reset when interface is down

PHYs supported by e100 re-starts auto-negotiation after writing to BMCR_RESET bit. This patch powers down PHY when the interface is down and reset is issued.

Signed-off-by: Jiang Wang <Jiang.Wang@riverbed.com>
---
 drivers/net/ethernet/intel/e100.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 9824e0a..b8e4910 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -2386,6 +2386,13 @@ static int e100_set_settings(struct net_device *netdev, struct ethtool_cmd *cmd)
 	err = mii_ethtool_sset(&nic->mii, cmd);
 	e100_exec_cb(nic, NULL, e100_configure);
 
+	if (!netif_running(netdev) && !(nic->flags & wol_magic)) {
+		uint16_t phy_data;
+		phy_data = mdio_read(nic->netdev, nic->mii.phy_id, MII_BMCR);
+		phy_data |= BMCR_PDOWN;
+		mdio_write(nic->netdev, nic->mii.phy_id, MII_BMCR, phy_data);
+	}
+
 	return err;
 }
 
--
1.7.1


------------------------------------------------------------------------------
Learn Windows Azure Live!  Tuesday, Dec 13, 2011
Microsoft is holding a special Learn Windows Azure training event for 
developers. It will provide a great way to learn Windows Azure and what it 
provides. You can attend the event by watching it streamed LIVE online.  
Learn more at http://p.sf.net/sfu/ms-windowsazure
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related

* [PATCH] fix sleeping while atomic problem in sock mem_cgroup.
From: Glauber Costa @ 2011-12-16  2:37 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Glauber Costa, Stephen Rothwell

Since we can't scan the proto_list to initialize sock cgroups, as it
holds a rwlock, and we also want to keep the code generic enough to
avoid calling the initialization functions of protocols directly,
I propose we keep the interested parties in a separate list. This list
is protected by a mutex so we can sleep and do the necessary allocations.

Also fixes a reference problem found by Randy Dunlap's randconfig.

Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
CC: Hiroyouki Kamezawa <kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
CC: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
CC: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
---
 include/linux/memcontrol.h   |    4 +++
 include/net/sock.h           |    5 +---
 include/net/tcp_memcontrol.h |    2 +
 mm/memcontrol.c              |   52 ++++++++++++++++++++++++++++++++++++++++++
 net/core/sock.c              |   48 +++++++++-----------------------------
 5 files changed, 70 insertions(+), 41 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 9b296ea..1edd0e3 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -390,6 +390,10 @@ enum {
 	OVER_LIMIT,
 };
 
+struct proto;
+void register_sock_cgroup(struct proto *prot);
+void unregister_sock_cgroup(struct proto *prot);
+
 #ifdef CONFIG_INET
 struct sock;
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
diff --git a/include/net/sock.h b/include/net/sock.h
index 6fe0dae..f8237a3 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -64,10 +64,6 @@
 #include <net/dst.h>
 #include <net/checksum.h>
 
-struct cgroup;
-struct cgroup_subsys;
-int mem_cgroup_sockets_init(struct cgroup *cgrp, struct cgroup_subsys *ss);
-void mem_cgroup_sockets_destroy(struct cgroup *cgrp, struct cgroup_subsys *ss);
 /*
  * This structure really needs to be cleaned up.
  * Most of it is for TCP, and not used by any of
@@ -858,6 +854,7 @@ struct proto {
 	void			(*destroy_cgroup)(struct cgroup *cgrp,
 						  struct cgroup_subsys *ss);
 	struct cg_proto		*(*proto_cgroup)(struct mem_cgroup *memcg);
+	struct list_head	cgroup_node;
 #endif
 };
 
diff --git a/include/net/tcp_memcontrol.h b/include/net/tcp_memcontrol.h
index 3512082..5aa7c4b 100644
--- a/include/net/tcp_memcontrol.h
+++ b/include/net/tcp_memcontrol.h
@@ -11,6 +11,8 @@ struct tcp_memcontrol {
 	int tcp_memory_pressure;
 };
 
+struct cgroup;
+struct cgroup_subsys;
 struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg);
 int tcp_init_cgroup(struct cgroup *cgrp, struct cgroup_subsys *ss);
 void tcp_destroy_cgroup(struct cgroup *cgrp, struct cgroup_subsys *ss);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7266202..6ee250d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4742,6 +4742,58 @@ static struct cftype kmem_cgroup_files[] = {
 	},
 };
 
+static DEFINE_MUTEX(cgroup_proto_list_lock);
+static LIST_HEAD(cgroup_proto_list);
+
+static int mem_cgroup_sockets_init(struct cgroup *cgrp, struct cgroup_subsys *ss)
+{
+	struct proto *proto;
+	int ret = 0;
+
+	mutex_lock(&cgroup_proto_list_lock);
+	list_for_each_entry(proto, &cgroup_proto_list, cgroup_node) {
+		if (proto->init_cgroup) {
+			ret = proto->init_cgroup(cgrp, ss);
+			if (ret)
+				goto out;
+		}
+	}
+
+	mutex_unlock(&cgroup_proto_list_lock);
+	return ret;
+out:
+	list_for_each_entry_continue_reverse(proto, &cgroup_proto_list, cgroup_node)
+		if (proto->destroy_cgroup)
+			proto->destroy_cgroup(cgrp, ss);
+	mutex_unlock(&cgroup_proto_list_lock);
+	return ret;
+}
+
+static void mem_cgroup_sockets_destroy(struct cgroup *cgrp, struct cgroup_subsys *ss)
+{
+	struct proto *proto;
+
+	mutex_lock(&cgroup_proto_list_lock);
+	list_for_each_entry_reverse(proto, &cgroup_proto_list, cgroup_node)
+		if (proto->destroy_cgroup)
+			proto->destroy_cgroup(cgrp, ss);
+	mutex_unlock(&cgroup_proto_list_lock);
+}
+
+void register_sock_cgroup(struct proto *prot)
+{
+	mutex_lock(&cgroup_proto_list_lock);
+	list_add(&prot->cgroup_node, &cgroup_proto_list);
+	mutex_unlock(&cgroup_proto_list_lock);
+}
+
+void unregister_sock_cgroup(struct proto *prot)
+{
+	mutex_lock(&cgroup_proto_list_lock);
+	list_del(&prot->cgroup_node);
+	mutex_unlock(&cgroup_proto_list_lock);
+}
+
 static int register_kmem_files(struct cgroup *cont, struct cgroup_subsys *ss)
 {
 	int ret = 0;
diff --git a/net/core/sock.c b/net/core/sock.c
index 5a6a906..3728b50 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -139,43 +139,6 @@
 static DEFINE_RWLOCK(proto_list_lock);
 static LIST_HEAD(proto_list);
 
-#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
-int mem_cgroup_sockets_init(struct cgroup *cgrp, struct cgroup_subsys *ss)
-{
-	struct proto *proto;
-	int ret = 0;
-
-	read_lock(&proto_list_lock);
-	list_for_each_entry(proto, &proto_list, node) {
-		if (proto->init_cgroup) {
-			ret = proto->init_cgroup(cgrp, ss);
-			if (ret)
-				goto out;
-		}
-	}
-
-	read_unlock(&proto_list_lock);
-	return ret;
-out:
-	list_for_each_entry_continue_reverse(proto, &proto_list, node)
-		if (proto->destroy_cgroup)
-			proto->destroy_cgroup(cgrp, ss);
-	read_unlock(&proto_list_lock);
-	return ret;
-}
-
-void mem_cgroup_sockets_destroy(struct cgroup *cgrp, struct cgroup_subsys *ss)
-{
-	struct proto *proto;
-
-	read_lock(&proto_list_lock);
-	list_for_each_entry_reverse(proto, &proto_list, node)
-		if (proto->destroy_cgroup)
-			proto->destroy_cgroup(cgrp, ss);
-	read_unlock(&proto_list_lock);
-}
-#endif
-
 /*
  * Each address family might have different locking rules, so we have
  * one slock key per address family:
@@ -2483,6 +2446,12 @@ int proto_register(struct proto *prot, int alloc_slab)
 	list_add(&prot->node, &proto_list);
 	assign_proto_idx(prot);
 	write_unlock(&proto_list_lock);
+
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
+	if (prot->proto_cgroup)
+		register_sock_cgroup(prot);
+#endif
+
 	return 0;
 
 out_free_timewait_sock_slab_name:
@@ -2510,6 +2479,11 @@ void proto_unregister(struct proto *prot)
 	list_del(&prot->node);
 	write_unlock(&proto_list_lock);
 
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
+	if (prot->proto_cgroup != NULL)
+		unregister_sock_cgroup(prot);
+#endif
+
 	if (prot->slab != NULL) {
 		kmem_cache_destroy(prot->slab);
 		prot->slab = NULL;
-- 
1.7.6.4

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

^ permalink raw reply related

* Re: [PATCH] tg3: Make the RSS indir tbl admin configurable
From: Ben Hutchings @ 2011-12-16  3:04 UTC (permalink / raw)
  To: Matt Carlson; +Cc: davem, netdev, Michael Chan
In-Reply-To: <1323996466-8139-1-git-send-email-mcarlson@broadcom.com>

On Thu, 2011-12-15 at 16:47 -0800, Matt Carlson wrote:
> This patch adds the ethtool callbacks necessary to change the rss
> indirection table from userspace.  When setting the indirection table
> through set_rxfh_indir, an indirection table size of zero is
> interpreted to mean that the admin wants to relinquish control of the
> table to the driver.  Should the number of interrupts change (e.g.
> across a close / open call, or through a reset), any indirection table
> values that exceed the number of RSS queues or interrupt vectors will
> be automatically scaled back to values within range.
> 
> Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

No I didn't!

Ben.

> Reviewed-by: Benjamin Li <benli@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/tg3.c |  126 ++++++++++++++++++++++++++++++++++-
>  drivers/net/ethernet/broadcom/tg3.h |    1 +
>  2 files changed, 126 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index 8bf11ca..87f70f6 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -8229,9 +8229,19 @@ void tg3_rss_init_indir_tbl(struct tg3 *tp)
>  
>  	if (tp->irq_cnt <= 2)
>  		memset(&tp->rss_ind_tbl[0], 0, sizeof(tp->rss_ind_tbl));
> -	else
> +	else if (tg3_flag(tp, USER_INDIR_TBL)) {
> +		/* Validate table against current IRQ count */
> +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++) {
> +			if (tp->rss_ind_tbl[i] >= tp->irq_cnt - 1) {
> +				/* Bring the index within range */
> +				tp->rss_ind_tbl[i] = tp->rss_ind_tbl[i] %
> +						     (tp->irq_cnt - 1);
> +			}
> +		}
> +	} else {
>  		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
>  			tp->rss_ind_tbl[i] = i % (tp->irq_cnt - 1);
> +	}
>  }
>  
>  void tg3_rss_write_indir_tbl(struct tg3 *tp)
> @@ -10719,6 +10729,117 @@ static int tg3_get_sset_count(struct net_device *dev, int sset)
>  	}
>  }
>  
> +static int tg3_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
> +			 u32 *rules __always_unused)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +
> +	if (!tg3_flag(tp, SUPPORT_MSIX))
> +		return -EOPNOTSUPP;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_GRXRINGS:
> +		if (netif_running(tp->dev))
> +			info->data = tp->irq_cnt;
> +		else {
> +			info->data = num_online_cpus();
> +			if (info->data > TG3_IRQ_MAX_VECS_RSS)
> +				info->data = TG3_IRQ_MAX_VECS_RSS;
> +		}
> +
> +		/* The first interrupt vector only
> +		 * handles link interrupts.
> +		 */
> +		info->data -= 1;
> +		return 0;
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int tg3_get_rxfh_indir(struct net_device *dev,
> +			      struct ethtool_rxfh_indir *indir)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +	int i;
> +
> +	if (!tg3_flag(tp, SUPPORT_MSIX))
> +		return -EOPNOTSUPP;
> +
> +	if (!indir->size) {
> +		indir->size = TG3_RSS_INDIR_TBL_SIZE;
> +		return 0;
> +	}
> +
> +	if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> +		return -EINVAL;
> +
> +	for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> +		indir->ring_index[i] = tp->rss_ind_tbl[i];
> +
> +	return 0;
> +}
> +
> +static int tg3_set_rxfh_indir(struct net_device *dev,
> +			      const struct ethtool_rxfh_indir *indir)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +	size_t i;
> +
> +	if (!tg3_flag(tp, SUPPORT_MSIX))
> +		return -EOPNOTSUPP;
> +
> +	if (!indir->size) {
> +		tg3_flag_clear(tp, USER_INDIR_TBL);
> +		tg3_rss_init_indir_tbl(tp);
> +	} else {
> +		int limit;
> +
> +		/* Validate size and indices */
> +		if (indir->size != TG3_RSS_INDIR_TBL_SIZE)
> +			return -EINVAL;
> +
> +		if (netif_running(dev))
> +			limit = tp->irq_cnt;
> +		else {
> +			limit = num_online_cpus();
> +			if (limit > TG3_IRQ_MAX_VECS_RSS)
> +				limit = TG3_IRQ_MAX_VECS_RSS;
> +		}
> +
> +		/* The first interrupt vector only
> +		 * handles link interrupts.
> +		 */
> +		limit -= 1;
> +
> +		/* Check the indices in the table.
> +		 * Leave the existing table unmodified
> +		 * if an error is detected.
> +		 */
> +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> +			if (indir->ring_index[i] >= limit)
> +				return -EINVAL;
> +
> +		tg3_flag_set(tp, USER_INDIR_TBL);
> +
> +		for (i = 0; i < TG3_RSS_INDIR_TBL_SIZE; i++)
> +			tp->rss_ind_tbl[i] = indir->ring_index[i];
> +	}
> +
> +	if (!netif_running(dev) || !tg3_flag(tp, ENABLE_RSS))
> +		return 0;
> +
> +	/* It is legal to write the indirection
> +	 * table while the device is running.
> +	 */
> +	tg3_full_lock(tp, 0);
> +	tg3_rss_write_indir_tbl(tp);
> +	tg3_full_unlock(tp);
> +
> +	return 0;
> +}
> +
>  static void tg3_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
>  {
>  	switch (stringset) {
> @@ -11949,6 +12070,9 @@ static const struct ethtool_ops tg3_ethtool_ops = {
>  	.get_coalesce		= tg3_get_coalesce,
>  	.set_coalesce		= tg3_set_coalesce,
>  	.get_sset_count		= tg3_get_sset_count,
> +	.get_rxnfc		= tg3_get_rxnfc,
> +	.get_rxfh_indir		= tg3_get_rxfh_indir,
> +	.set_rxfh_indir		= tg3_set_rxfh_indir,
>  };
>  
>  static void __devinit tg3_get_eeprom_size(struct tg3 *tp)
> diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
> index aea8f72..4800595 100644
> --- a/drivers/net/ethernet/broadcom/tg3.h
> +++ b/drivers/net/ethernet/broadcom/tg3.h
> @@ -2932,6 +2932,7 @@ enum TG3_FLAGS {
>  	TG3_FLAG_APE_HAS_NCSI,
>  	TG3_FLAG_4K_FIFO_LIMIT,
>  	TG3_FLAG_RESET_TASK_PENDING,
> +	TG3_FLAG_USER_INDIR_TBL,
>  	TG3_FLAG_5705_PLUS,
>  	TG3_FLAG_IS_5788,
>  	TG3_FLAG_5750_PLUS,

-- 
Ben Hutchings, Staff Engineer, Solarflare
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


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