Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 03/10] mm: Add support for a filesystem to control swap files
From: Christoph Hellwig @ 2011-09-09 13:36 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Christoph Hellwig, Linux-MM, Linux-Netdev, Linux-NFS, LKML,
	Andrew Morton, David Miller, Trond Myklebust, Neil Brown,
	Peter Zijlstra
In-Reply-To: <20110909131550.GV14369@suse.de>

On Fri, Sep 09, 2011 at 02:15:50PM +0100, Mel Gorman wrote:
> 
> I confess I haven't investigated this direction at
> all yet.  Is it correct that your previous objection was
> http://linux.derkeiler.com/Mailing-Lists/Kernel/2009-10/msg00455.html
> and the direct-IO patchset you were thinking of was
> http://copilotco.com/mail-archives/linux-kernel.2009/msg87176.html ?

Yes.

> If so, are you suggesting that instead of swap_readpage and
> swap_writepage I look into what is required for swap to use ->readpage
> method and ->direct_IO aops?

The equivalent of ->direct_IO should be used for both reads and writes.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v2 3/5] SUNRPC: make RPC service dependable on rpcbind clients creation
From: Jeff Layton @ 2011-09-09 14:07 UTC (permalink / raw)
  To: Stanislav Kinsbursky
  Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA, xemul-bzQdu9zFT3WakBO8gow8eQ,
	neilb-l3A5Bk7waGM, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20110909120844.13697.48102.stgit-bi+AKbBUZKagILUCTcTcHdKyNwTtLsGr@public.gmane.org>

On Fri, 09 Sep 2011 16:08:44 +0400
Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org> wrote:

> Create rcbind clients or increase rpcbind users counter during RPC service
> creation and decrease this counter (and possibly destroy those clients) on RPC
> service destruction.
> 
> Signed-off-by: Stanislav Kinsbursky <skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> 
> ---
>  include/linux/sunrpc/clnt.h |    2 ++
>  net/sunrpc/rpcb_clnt.c      |    2 +-
>  net/sunrpc/svc.c            |   13 +++++++++++--
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index db7bcaf..65a8115 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -135,10 +135,12 @@ void		rpc_shutdown_client(struct rpc_clnt *);
>  void		rpc_release_client(struct rpc_clnt *);
>  void		rpc_task_release_client(struct rpc_task *);
>  
> +int		rpcb_create_local(void);
>  int		rpcb_register(u32, u32, int, unsigned short);
>  int		rpcb_v4_register(const u32 program, const u32 version,
>  				 const struct sockaddr *address,
>  				 const char *netid);
> +void		rpcb_put_local(void);
>  void		rpcb_getport_async(struct rpc_task *);
>  
>  void		rpc_call_start(struct rpc_task *);
> diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
> index b4cc0f1..437ec60 100644
> --- a/net/sunrpc/rpcb_clnt.c
> +++ b/net/sunrpc/rpcb_clnt.c
> @@ -318,7 +318,7 @@ out:
>   * Returns zero on success, otherwise a negative errno value
>   * is returned.
>   */
> -static int rpcb_create_local(void)
> +int rpcb_create_local(void)
>  {
>  	static DEFINE_MUTEX(rpcb_create_local_mutex);
>  	int result = 0;
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 6a69a11..9095c0e 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -367,8 +367,11 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>  	unsigned int xdrsize;
>  	unsigned int i;
>  
> -	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> +	if (rpcb_create_local() < 0)
>  		return NULL;
> +
> +	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> +		goto out_err;
>  	serv->sv_name      = prog->pg_name;
>  	serv->sv_program   = prog;
>  	serv->sv_nrthreads = 1;
> @@ -403,7 +406,7 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>  			GFP_KERNEL);
>  	if (!serv->sv_pools) {
>  		kfree(serv);
> -		return NULL;
> +		goto out_err;
>  	}
>  
>  	for (i = 0; i < serv->sv_nrpools; i++) {
> @@ -423,6 +426,10 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>  	svc_unregister(serv);
>  
>  	return serv;
> +
> +out_err:
> +	rpcb_put_local(); 
> +	return NULL;
>  }
>  
>  struct svc_serv *
> @@ -491,6 +498,8 @@ svc_destroy(struct svc_serv *serv)
>  	svc_unregister(serv);
>  	kfree(serv->sv_pools);
>  	kfree(serv);
> +
> +	rpcb_put_local();
>  }
>  EXPORT_SYMBOL_GPL(svc_destroy);
>  
> 

I don't get it -- what's the advantage of creating rpcbind clients in
__svc_create vs. the old way of creating them just before we plan to
use them?

With this scheme, won't we end up creating rpcbind sockets even when we
don't need them? For instance, if I create a callback socket for NFSv4
then I don't really need to talk to rpcbind. With this patch I'll still
get the rpcbind clients created though.

It would seem to me to make more sense to create the rpcbind clients
somewhere closer to svc_setup_socket, and only if SVC_SOCK_ANONYMOUS is
not set.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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

* [PATCH] RDSRDMA: Fix to PAGE_MASK interpretation
From: Venkat Venkatsubra @ 2011-09-09 14:35 UTC (permalink / raw)
  To: Netdev, Rds-Devel; +Cc: Davem


On 08/29/2011 02:28 PM, Jonathan Lallinger wrote:
> The RDS_RDMA rds_iw_map_scatterlist function assumed PAGE_MASK was the bitwise
> compliment of what it actually is. This problem was introduced in commit
> 404bb72a56e553febe1055f98347a7a3e3145759 when the variable dma_mask was replaced
> by PAGE_MASK, however dma_mask represented the compliment of what PAGE_MASK
> represents.
>
> This fix corrects the logic by flipping the compliments on all uses of PAGE_MASK
> int rds_iw_map_scatterlist.
>
> Signed-off by: Jonathan Lallinger<jonathan@ogc.us>

Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>

Venkat

^ permalink raw reply

* [PATCH] RDSRDMA: Fix cleanup of rds_iw_mr_pool
From: Venkat Venkatsubra @ 2011-09-09 14:37 UTC (permalink / raw)
  To: Netdev, Rds-Devel; +Cc: Davem


On 09/08/2011 01:04 PM, Jonathan Lallinger wrote:
> In the rds_iw_mr_pool struct the free_pinned field keeps track of memory pinned
> by free MRs. While this field is incremented properly upon allocation, it is never
> decremented upon unmapping. This would cause the rds_rdma module to crash the
> kernel upon unloading, by triggering the BUG_ON in the rds_iw_destroy_mr_pool
> function.
>
> This change keeps track of the MRs that become unpinned, so that free_pinned
> can be decremented appropriately.
>
> Signed-off-by: Jonathan Lallinger<jonathan@ogc.us>
> Signed-off-by: Steve Wise<swise@ogc.us>

Signed-off-by: Venkat Venkatsubra<venkat.x.venkatsubra@oracle.com>

Venkat

^ permalink raw reply

* [PATCH net-next] af_unix: use fragments in unix_dgram_sendmsg()
From: Eric Dumazet @ 2011-09-09 14:38 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Andi Kleen, Tim Chen

unix_dgram_sendmsg() currently builds basic skbs, with a possible huge
head.

This patch uses up to MAX_SKB_FRAGS fragments to reduce the head size.

This lowers memory pressure on high order LOWMEM pages
(frags pages can be allocated from HIGHMEM).

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/unix/af_unix.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 1bd4ecf..58570f5 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1420,6 +1420,7 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock,
 	long timeo;
 	struct scm_cookie tmp_scm;
 	int max_level;
+	int fragslen = 0;
 
 	if (NULL == siocb->scm)
 		siocb->scm = &tmp_scm;
@@ -1453,18 +1454,28 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock,
 	if (len > sk->sk_sndbuf - 32)
 		goto out;
 
-	skb = sock_alloc_send_skb(sk, len, msg->msg_flags&MSG_DONTWAIT, &err);
+	/* try to use fragments to get order-0 header allocation */
+	if (len > SKB_MAX_ORDER(0, 0))
+		fragslen = min_t(size_t,
+				 len - SKB_MAX_ORDER(0, 0),
+				 MAX_SKB_FRAGS * PAGE_SIZE);
+
+	skb = sock_alloc_send_pskb(sk, len - fragslen, fragslen,
+				   msg->msg_flags & MSG_DONTWAIT, &err);
 	if (skb == NULL)
 		goto out;
 
+	skb_put(skb, len - fragslen);
+	skb->data_len = fragslen;
+	skb->len = len;
+
 	err = unix_scm_to_skb(siocb->scm, skb, true, true);
 	if (err < 0)
 		goto out_free;
 	max_level = err + 1;
 	unix_get_secdata(siocb->scm, skb);
 
-	skb_reset_transport_header(skb);
-	err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
+	err = skb_copy_datagram_from_iovec(skb, 0, msg->msg_iov, 0, len);
 	if (err)
 		goto out_free;
 

^ permalink raw reply related

* Re: [PATCH 2/2] GRETH: avoid overwrite IP-stack's IP-frags checksum
From: Daniel Hellstrom @ 2011-09-09 14:41 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, netdev, kristoffer
In-Reply-To: <1315574967.2294.28.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

On 09/09/2011 03:29 PM, Eric Dumazet wrote:
> Le jeudi 08 septembre 2011 à 15:14 +0200, Daniel Hellstrom a écrit :
>> The GRETH GBIT core does not do checksum offloading for IP
>> segmentation. This patch adds a check in the xmit function to
>> determine if the stack has calculated the checksum for us.
>>
>> Signed-off-by: Daniel Hellstrom<daniel@gaisler.com>
>> ---
>>   drivers/net/greth.c |    7 +++++--
>>   1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/greth.c b/drivers/net/greth.c
>> index 9d39fb9..27ba855 100644
>> --- a/drivers/net/greth.c
>> +++ b/drivers/net/greth.c
>> @@ -489,7 +489,8 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
>>   	if (nr_frags != 0)
>>   		status = GRETH_TXBD_MORE;
>>
>> -	status |= GRETH_TXBD_CSALL;
>> +	if (skb->ip_summed == CHECKSUM_PARTIAL)
>> +		status |= GRETH_TXBD_CSALL;
>>   	status |= skb_headlen(skb)&  GRETH_BD_LEN;
>>   	if (greth->tx_next == GRETH_TXBD_NUM_MASK)
>>   		status |= GRETH_BD_WR;
>> @@ -512,7 +513,9 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
>>   		greth->tx_skbuff[curr_tx] = NULL;
>>   		bdp = greth->tx_bd_base + curr_tx;
>>
>> -		status = GRETH_TXBD_CSALL | GRETH_BD_EN;
>> +		status = GRETH_BD_EN;
>> +		if (skb->ip_summed == CHECKSUM_PARTIAL)
>> +			status | GRETH_TXBD_CSALL;
> 		typo here ?
Hi,

You're right, I had a missed space there so the code-style check failed at that line, then I fixed that by adding a space... but must have screwed it up. Will repost patch.

Thanks,
Daniel



>
>>   		status |= frag->size&  GRETH_BD_LEN;
>>
>>   		/* Wrap around descriptor ring */
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

^ permalink raw reply

* Re: [PATCH net-next-2.6] can/sja1000: driver for PEAK PCAN PCI/PCIe cards
From: Oliver Hartkopp @ 2011-09-09 14:42 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: SocketCAN Core Mailing List, Netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-g4cQ8AsIbFbL9ATBNaCtXw, Thomas Wiedemann
In-Reply-To: <4E692080.9020801-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Hello Wolfgang,

many thanks for providing this patch!

On 09/08/11 22:07, Wolfgang Grandegger wrote:

> This patch adds the "peak_pci" driver for the PCAN PCI/PCIe cards (1, 2, 3
> or 4 channels) from PEAK Systems (http://www.peak-system.com).


Yes - that fits, but ...


> +#define PEAK_PCI_VENDOR_ID	0x001C	/* The PCI device and vendor IDs */
> +#define PEAK_PCI_DEVICE_ID	0x0001	/* for PCI / PCIe slot cards */
> +#define PEAK_PCIE_DEVICE_ID	0x0002	/* for PCIExpress cards */
> +
> +static const u16 peak_pci_icr_masks[] = {0x02, 0x01, 0x40, 0x80};
> +
> +static DEFINE_PCI_DEVICE_TABLE(peak_pci_tbl) = {
> +	{PEAK_PCI_VENDOR_ID, PEAK_PCI_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
> +	{PEAK_PCI_VENDOR_ID, PEAK_PCIE_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
> +	{0,}
> +};


The problem is the support of the PCAN ExpressCard (device ID 0x0002) here.

This commit

   https://lists.berlios.de/pipermail/socketcan-commit/2010-June/000814.html

should be reverted, as you can use the PCAN-ExpressCard (IPEH-00300[12]) only
with a hardware patch, as we need some I2C initialisations to enable the CAN
transceiver (and the funky status LEDs). See at:

http://anonscm.debian.org/gitweb/?p=collab-maint/peak-linux-driver.git;a=blob;f=driver/src/pcan_pciec.c;h=87ce0bda41aa0a6dda8284d48237b1ef7c678e11;hb=HEAD#l36

I'll also test your driver on Monday.

Tnx & best regards,
Oliver

^ permalink raw reply

* Re: [PATCH 02/15] user ns: setns: move capable checks into per-ns attach helper
From: Serge E. Hallyn @ 2011-09-09 14:56 UTC (permalink / raw)
  To: Matt Helsley
  Cc: akpm, segooon, linux-kernel, netdev, containers, dhowells,
	ebiederm, rdunlap
In-Reply-To: <20110904015140.GB32295@count0.beaverton.ibm.com>

Quoting Matt Helsley (matthltc@us.ibm.com):
> On Fri, Sep 02, 2011 at 07:56:27PM +0000, Serge Hallyn wrote:
> > From: "Serge E. Hallyn" <serge@hallyn.com>
> 
> I was confused about this patch until I realized that you're not
> simply "moving" the capability checks but "distributing" them. Then
> you're showing that you'll soon change some to nsown_capable() or
> ns_capable() using the strange cpp pattern in the snippet below.
> 
> At least I think that's what you intended. A commit message would
> help :).

Yes, sorry - Eric convinced me several times to be more conservative in
the patch, and I failed to fix the commit msg when squashing the
resulting patches.  How about the following:

======

user ns: update capable calls when cloning and attaching namespaces

Distribute the capable() checks at ns attach into the namespace-specific
attach handler.

Note the fact that the capable() checks will be changed to targeted
checks at both namespace clone and attach methods, but don't actually
make that change yet.  Until that trigger is pulled, you must have
the capabilities targeted toward the initial user namespace in order to
do any of these actions, meaning that a task in a child user namespace
cannot do them.  Once we pull the trigger, a task in a child user
namespace will be able to clone new namespaces if it is privileged in
its own user namespace, and attach to existing namespaces to which it
has privilege.

======

Thanks for taking a look, Matt!

-serge

^ permalink raw reply

* [PATCH 2/2 v2] GRETH: avoid overwrite IP-stack's IP-frags checksum
From: Daniel Hellstrom @ 2011-09-09 15:17 UTC (permalink / raw)
  To: davem; +Cc: netdev, kristoffer, eric.dumazet

The GRETH GBIT core does not do checksum offloading for IP
segmentation. This patch adds a check in the xmit function to
determine if the stack has calculated the checksum for us.

Signed-off-by: Daniel Hellstrom <daniel@gaisler.com>
---
 drivers/net/greth.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/greth.c b/drivers/net/greth.c
index 22e24a3..6775c24 100644
--- a/drivers/net/greth.c
+++ b/drivers/net/greth.c
@@ -489,7 +489,8 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
 	if (nr_frags != 0)
 		status = GRETH_TXBD_MORE;
 
-	status |= GRETH_TXBD_CSALL;
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		status |= GRETH_TXBD_CSALL;
 	status |= skb_headlen(skb) & GRETH_BD_LEN;
 	if (greth->tx_next == GRETH_TXBD_NUM_MASK)
 		status |= GRETH_BD_WR;
@@ -512,7 +513,9 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev)
 		greth->tx_skbuff[curr_tx] = NULL;
 		bdp = greth->tx_bd_base + curr_tx;
 
-		status = GRETH_TXBD_CSALL | GRETH_BD_EN;
+		status = GRETH_BD_EN;
+		if (skb->ip_summed == CHECKSUM_PARTIAL)
+			status |= GRETH_TXBD_CSALL;
 		status |= frag->size & GRETH_BD_LEN;
 
 		/* Wrap around descriptor ring */
-- 
1.5.4

^ permalink raw reply related

* Re: [PATCH net-next-2.6] can/sja1000: driver for PEAK PCAN PCI/PCIe cards
From: Wolfgang Grandegger @ 2011-09-09 15:20 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: SocketCAN Core Mailing List, Netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-g4cQ8AsIbFbL9ATBNaCtXw, Thomas Wiedemann
In-Reply-To: <4E6A25EE.3000501-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

Hi Oliver,

On 09/09/2011 04:42 PM, Oliver Hartkopp wrote:
> Hello Wolfgang,
> 
> many thanks for providing this patch!
> 
> On 09/08/11 22:07, Wolfgang Grandegger wrote:
> 
>> This patch adds the "peak_pci" driver for the PCAN PCI/PCIe cards (1, 2, 3
>> or 4 channels) from PEAK Systems (http://www.peak-system.com).
> 
> 
> Yes - that fits, but ...
> 
> 
>> +#define PEAK_PCI_VENDOR_ID	0x001C	/* The PCI device and vendor IDs */
>> +#define PEAK_PCI_DEVICE_ID	0x0001	/* for PCI / PCIe slot cards */
>> +#define PEAK_PCIE_DEVICE_ID	0x0002	/* for PCIExpress cards */
>> +
>> +static const u16 peak_pci_icr_masks[] = {0x02, 0x01, 0x40, 0x80};
>> +
>> +static DEFINE_PCI_DEVICE_TABLE(peak_pci_tbl) = {
>> +	{PEAK_PCI_VENDOR_ID, PEAK_PCI_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
>> +	{PEAK_PCI_VENDOR_ID, PEAK_PCIE_DEVICE_ID, PCI_ANY_ID, PCI_ANY_ID,},
>> +	{0,}
>> +};
> 
> 
> The problem is the support of the PCAN ExpressCard (device ID 0x0002) here.
> 
> This commit
> 
>    https://lists.berlios.de/pipermail/socketcan-commit/2010-June/000814.html
> 
> should be reverted, as you can use the PCAN-ExpressCard (IPEH-00300[12]) only
> with a hardware patch, as we need some I2C initialisations to enable the CAN
> transceiver (and the funky status LEDs). See at:
> 
> http://anonscm.debian.org/gitweb/?p=collab-maint/peak-linux-driver.git;a=blob;f=driver/src/pcan_pciec.c;h=87ce0bda41aa0a6dda8284d48237b1ef7c678e11;hb=HEAD#l36

OK, I see, I will revert then. I already realized the I2C stuff for the
PCAN-ExpressCard. Obviously Thomas does not use that card but a PCIe
slot cards

> I'll also test your driver on Monday.

Thanks,

Wolfgang.

^ permalink raw reply

* Re: [net-next-2.6 PATCH 0/3 RFC] macvlan: MAC Address filtering support for passthru mode
From: Roopa Prabhu @ 2011-09-09 16:21 UTC (permalink / raw)
  To: Sridhar Samudrala
  Cc: Michael S. Tsirkin, netdev, dragos.tatulea, arnd, dwang2, benve,
	kaber, davem, eric.dumazet, mchan, kvm
In-Reply-To: <4E699556.6050809@us.ibm.com>




On 9/8/11 9:25 PM, "Sridhar Samudrala" <sri@us.ibm.com> wrote:

> On 9/8/2011 8:00 PM, Roopa Prabhu wrote:
>> 
>> 
>> On 9/8/11 12:33 PM, "Michael S. Tsirkin"<mst@redhat.com>  wrote:
>> 
>>> On Thu, Sep 08, 2011 at 12:23:56PM -0700, Roopa Prabhu wrote:
>>>>> I think the main usecase for passthru mode is to assign a SR-IOV VF to
>>>>> a single guest.
>>>>> 
>>>> Yes and for the passthru usecase this patch should be enough to enable
>>>> filtering in hw (eventually like I indicated before I need to fix vlan
>>>> filtering too).
>>> So with filtering in hw, and in sriov VF case, VFs
>>> actually share a filtering table. How will that
>>> be partitioned?
>> AFAIK, though it might maintain a single filter table space in hw, hw does
>> know which filter belongs to which VF. And the OS driver does not need to do
>> anything special. The VF driver exposes a VF netdev. And any uc/mc addresses
>> registered with a VF netdev are registered with the hw by the driver. And hw
>> will filter and send only pkts that the VF has expressed interest in.
> Does your NIC & driver support adding multiple mac addresses to a VF?
> I have tried a few other SR-IOV NICs sometime back and they didn't
> support this feature.

Yes our nic does. I thought Intel's also does (see ixgbevf_set_rx_mode).
Though I have not really tried using it on an Intel card. I think most cards
should at the least support multicast filters.

If the lower dev does not support unicast filtering, dev_uc_add(lowerdev,..)
puts the lower dev in promiscous mode. Though..i think I can chcek this
before hand in macvlan_open and put the lowerdev in promiscuous mode if it
does not support filtering.

> 
> Currently, we don't have an interface to add multiple mac addresses to a
> netdev other than an
> indirect way of creating a macvlan /if on top of it.

Yes I think so. I have been using only macvlan to test.

Thanks,
Roopa


^ permalink raw reply

* Re: [net-next-2.6 PATCH 0/3 RFC] macvlan: MAC Address filtering support for passthru mode
From: Roopa Prabhu @ 2011-09-09 16:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, dragos.tatulea, arnd, dwang2, benve, kaber, sri, davem,
	eric.dumazet, mchan, kvm
In-Reply-To: <20110909055530.GA4647@redhat.com>




On 9/8/11 10:55 PM, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Sep 08, 2011 at 07:53:11PM -0700, Roopa Prabhu wrote:
>>>> Phase 1: Goal: Enable hardware filtering for all macvlan modes
>>>>     - In macvlan passthru mode the single guest virtio-nic connected will
>>>>       receive traffic that he requested for
>>>>     - In macvlan non-passthru mode all guest virtio-nics sharing the
>>>>       physical nic will see all other guest traffic
>>>>       but the filtering at guest virtio-nic
>>> 
>>> I don't think guests currently filter anything.
>>> 
>> I was referring to Qemu-kvm virtio-net in
>> virtion_net_receive->receive_filter. I think It only passes pkts that the
>> guest OS is interested. It uses the filter table that I am passing to
>> macvtap in this patch.
> 
> This happens after userspace thread gets woken up and data
> is copied there. So relying on filtering at that level is
> going to be very inefficient on a system with
> multiple active guests. Further, and for that reason, vhost-net
> doesn't do filtering at all, relying on the backends
> to pass it correct packets.

Ok thanks for the info. So in which case, phase 1 is best for PASSTHRU mode
and for non-PASSTHRU when there is a single guest connected to a VF.
For non-PASSTHRU multi guest sharing the same VF, Phase 1 is definitely
better than putting the VF in promiscuous mode.
But to address the concern you mention above, in phase 2 when we have more
than one guest sharing the VF, we will have to add filter lookup in macvlan
to filter pkts for each guest. This will need some performance tests too.

Will start investigating the netlink interface comments for phase 1 first.

Thanks!
-Roopa


^ permalink raw reply

* Re: [PATCH v2 3/5] SUNRPC: make RPC service dependable on rpcbind clients creation
From: Stanislav Kinsbursky @ 2011-09-09 16:41 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Trond.Myklebust@netapp.com, linux-nfs@vger.kernel.org,
	Pavel Emelianov, neilb@suse.de, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, bfields@fieldses.org,
	davem@davemloft.net
In-Reply-To: <20110909100745.7e2e8bf9@corrin.poochiereds.net>

09.09.2011 18:07, Jeff Layton пишет:
> On Fri, 09 Sep 2011 16:08:44 +0400
> Stanislav Kinsbursky<skinsbursky@parallels.com>  wrote:
>
>> Create rcbind clients or increase rpcbind users counter during RPC service
>> creation and decrease this counter (and possibly destroy those clients) on RPC
>> service destruction.
>>
>> Signed-off-by: Stanislav Kinsbursky<skinsbursky@parallels.com>
>>
>> ---
>>   include/linux/sunrpc/clnt.h |    2 ++
>>   net/sunrpc/rpcb_clnt.c      |    2 +-
>>   net/sunrpc/svc.c            |   13 +++++++++++--
>>   3 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
>> index db7bcaf..65a8115 100644
>> --- a/include/linux/sunrpc/clnt.h
>> +++ b/include/linux/sunrpc/clnt.h
>> @@ -135,10 +135,12 @@ void		rpc_shutdown_client(struct rpc_clnt *);
>>   void		rpc_release_client(struct rpc_clnt *);
>>   void		rpc_task_release_client(struct rpc_task *);
>>
>> +int		rpcb_create_local(void);
>>   int		rpcb_register(u32, u32, int, unsigned short);
>>   int		rpcb_v4_register(const u32 program, const u32 version,
>>   				 const struct sockaddr *address,
>>   				 const char *netid);
>> +void		rpcb_put_local(void);
>>   void		rpcb_getport_async(struct rpc_task *);
>>
>>   void		rpc_call_start(struct rpc_task *);
>> diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
>> index b4cc0f1..437ec60 100644
>> --- a/net/sunrpc/rpcb_clnt.c
>> +++ b/net/sunrpc/rpcb_clnt.c
>> @@ -318,7 +318,7 @@ out:
>>    * Returns zero on success, otherwise a negative errno value
>>    * is returned.
>>    */
>> -static int rpcb_create_local(void)
>> +int rpcb_create_local(void)
>>   {
>>   	static DEFINE_MUTEX(rpcb_create_local_mutex);
>>   	int result = 0;
>> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
>> index 6a69a11..9095c0e 100644
>> --- a/net/sunrpc/svc.c
>> +++ b/net/sunrpc/svc.c
>> @@ -367,8 +367,11 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>>   	unsigned int xdrsize;
>>   	unsigned int i;
>>
>> -	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
>> +	if (rpcb_create_local()<  0)
>>   		return NULL;
>> +
>> +	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
>> +		goto out_err;
>>   	serv->sv_name      = prog->pg_name;
>>   	serv->sv_program   = prog;
>>   	serv->sv_nrthreads = 1;
>> @@ -403,7 +406,7 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>>   			GFP_KERNEL);
>>   	if (!serv->sv_pools) {
>>   		kfree(serv);
>> -		return NULL;
>> +		goto out_err;
>>   	}
>>
>>   	for (i = 0; i<  serv->sv_nrpools; i++) {
>> @@ -423,6 +426,10 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
>>   	svc_unregister(serv);
>>
>>   	return serv;
>> +
>> +out_err:
>> +	rpcb_put_local();
>> +	return NULL;
>>   }
>>
>>   struct svc_serv *
>> @@ -491,6 +498,8 @@ svc_destroy(struct svc_serv *serv)
>>   	svc_unregister(serv);
>>   	kfree(serv->sv_pools);
>>   	kfree(serv);
>> +
>> +	rpcb_put_local();
>>   }
>>   EXPORT_SYMBOL_GPL(svc_destroy);
>>
>>
>
> I don't get it -- what's the advantage of creating rpcbind clients in
> __svc_create vs. the old way of creating them just before we plan to
> use them?
>

The main problem here is not in creation, but in destroying those clients.
Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.

> With this scheme, won't we end up creating rpcbind sockets even when we
> don't need them? For instance, if I create a callback socket for NFSv4
> then I don't really need to talk to rpcbind. With this patch I'll still
> get the rpcbind clients created though.
>

Yep, you right. This is not a real problem from my pow.
But I'll think about how to avoid this creation for nfs callbacks.

> It would seem to me to make more sense to create the rpcbind clients
> somewhere closer to svc_setup_socket, and only if SVC_SOCK_ANONYMOUS is
> not set.
>

Probably. Will try to find better place.

-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply

* Re: Octeon crash in virt_to_page(&core0_stack_variable)
From: David Daney @ 2011-09-09 16:59 UTC (permalink / raw)
  To: Cosmin Ratiu; +Cc: linux-mips, netdev
In-Reply-To: <201109091623.29000.cratiu@ixiacom.com>

On 09/09/2011 06:23 AM, Cosmin Ratiu wrote:
> Hello,
>
> I've been investigating a strange crash and I wanted to ask for your help.
> The crash happens when virt_to_page is called with an address from the softirq
> stack of core 0 on Cavium Octeon. It may happen on other MIPS processors as
> well, but I'm not sure.
>
> I've attached a simple kernel module to demonstrate the problem and the output
> of dmesg + the crash. Two seconds after inserting the module, the kernel
> should crash.
>
>  From what I've dug up in the kernel sources, it seems the stack for the first
> idle task resides in the data segment (mapped in kseg2) while the rest are
> allocated with kmalloc in __cpu_up() and reside in a different area (CAC_BASE
> upwards).
> It seems virt_to_phys produces bogus results for kseg2 and after that,
> virt_to_page crashes trying to access invalid memory.
>
> This problem was discovered when doing BGP traffic with the TCP MD5 option
> activated, where the following call chain caused a crash:
>
>   * tcp_v4_rcv
>   *  tcp_v4_timewait_ack
>   *   tcp_v4_send_ack ->  follow stack variable rep.th
>   *    tcp_v4_md5_hash_hdr
>   *     tcp_md5_hash_header
>   *      sg_init_one
>   *       sg_set_buf
>   *        virt_to_page
>
> I noticed that tcp_v4_send_reset uses a similar stack variable and also calls
> tcp_v4_md5_hash_hdr, so it has the same problem.
>
> I don't fully understand octeon mm details, so I wanted to bring up this issue
> in order to find a proper fix.
> To avoid the problem, I've implemented a quick hack to declare those variables
> percpu instead of on the stack, so they would also reside in CAC_BASE upwards.
> I've attached a patch against 2.6.32 for reference.
>
> Cosmin.
>
>
[...]
> [ 2040.300/0] Call Trace:
> [ 2040.300/0] [<ffffffffc123a054>] vcrash+0x54/0x80 [vcrash]
> [ 2040.300/0] [<ffffffffc0065f28>] run_timer_softirq+0x198/0x23c
> [ 2040.300/0] [<ffffffffc00609e0>] __do_softirq+0xd8/0x188

                   ^^^^^^^^^ CKSEG2 addresses detected!

You are using the out-of-tree mapped kernel patch which mucks about with 
the implementation of virt_to_phys().

Can you reproduce the TCP related crash in an unpatched kernel?

If not, then it would point to problems in the out-of-tree patches you 
have applied.

David Daney

^ permalink raw reply

* Using gretap to tunnel layer 2 traffic
From: John H @ 2011-09-09 17:25 UTC (permalink / raw)
  To: netdev@vger.kernel.org

I am attempting to tunnel Layer 2 traffic through a gretap device, 
while encrypting the GRE tunnel with IPsec. My test environment is as follows:

10.0.1.1                                          10.0.1.2
client_a <--> server_left <==> server_right <---> client_b
                      gretap/IPsec
                      

On the servers, I have two VLANs per server, corresponding to the unencrypted
and encrypted interfaces.  On each server, the unencrypted VLAN is 
bridged with the gretap device.  All VLANs and physical devices have MTUs of 
1500.  The gretap device has a resultant MTU of 1462, thereby causing the 
bridge device to have an MTU of 1462.

Traffic for the most part works as it is expected to behave. However, 
packets are dropped when client_a sends an ICMP packet to client_b which 
has an MTU less than client_a's device MTU but larger than server_left's 
MTU. I suspect other protocols would behave similarly (silently dropping
packets).  I an running "ping -c 1 -s 1450 10.0.1.2" on client_a, this results
in an ICMP packet being sent with an MTU of 1478.

An MTU of 1478 is larger than the bridge device's MTU of 1462, causing the 
packet to be silently discarded per net/bridge/br_forward.c 
in function br_dev_queue_push_xmit:

int br_dev_queue_push_xmit(struct sk_buff *skb)
{
    /* drop mtu oversized packets except gso */
    if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))
        kfree_skb(skb);
    else {
    ....
    
If the gretap device supported GSO, I suspect that this would not be a
problem. (ethtool -k gretapLeftRight states that GSO/GRO/LRO is not 
supported)

Function br_dev_queue_push_xmit eventually calls ipgre_tunnel_xmit, if the 
packet is not dropped.

I would think that br_dev_queue_push_xmit should call ipgre_tunnel_xmit
regardless of the device MTU and ipgre_tunnel_xmit would handle packet
fragmentation internally, but it never has the chance.

I have tried allowing all packets through br_dev_queue_push_xmit
and explicitly setting the Don't Frament field to 0 in ipgre_tunnel_xmit,
but this didn't solve problem.

Given that this would be tunneling Layer 2 traffic, it wouldn't make sense
to send an ICMP_FRAG_NEEDED response from the bridge.

The real question is, however, why is any client able to send a single ICMP
packet with size 1478 bytes when one of the hops along the way only 
supports 1462 bytes per its MTU? Shouldn't this have been negotiated 
beforehand?

^ permalink raw reply

* Re: [PATCH net-next] af_unix: fix use after free in unix_stream_recvmsg()
From: Tim Chen @ 2011-09-09 10:39 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: sedat.dilek, Yan, Zheng, Yan, Zheng, netdev@vger.kernel.org,
	davem@davemloft.net, sfr@canb.auug.org.au, jirislaby@gmail.com,
	Shi, Alex, Valdis Kletnieks
In-Reply-To: <1315555109.2294.9.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>

On Fri, 2011-09-09 at 09:58 +0200, Eric Dumazet wrote:
> Le vendredi 09 septembre 2011 à 08:51 +0200, Eric Dumazet a écrit :
> 
> > Now we have to fix a bug in unix_stream_recvmsg() as well.
> > 
> > consume_skb() call actually releases pid/cred references, and we can use
> > them after their eventual freeing.
> > 
> > Keep also in mind that receiver can provides a too short user buffer,
> > and skb can be put back to head of sk_receive_queue
> > 
> 
> Here is the patch to address this point.
> 
> Apply it after (af_unix: Fix use-after-free crashes)
> 
> I can make a combo patch once everybody agrees.
> 
> [PATCH net-next] af_unix: fix use after free in unix_stream_recvmsg()
> 
> Commit 0856a30409 (Scm: Remove unnecessary pid & credential references
> in Unix socket's send and receive path) introduced an use-after-free
> bug in unix_stream_recvmsg().
> 
> We should call consume_skb(skb) only after our possible use of pid/cred.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/unix/af_unix.c |   13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index c8a08ba..1bd4ecf 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1873,6 +1873,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	int target;
>  	int err = 0;
>  	long timeo;
> +	struct sk_buff *skb;
>  
>  	err = -EINVAL;
>  	if (sk->sk_state != TCP_ESTABLISHED)
> @@ -1904,7 +1905,6 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  	do {
>  		int chunk;
> -		struct sk_buff *skb;
>  
>  		unix_state_lock(sk);
>  		skb = skb_dequeue(&sk->sk_receive_queue);
> @@ -1949,6 +1949,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  			if ((UNIXCB(skb).pid  != siocb->scm->pid) ||
>  			    (UNIXCB(skb).cred != siocb->scm->cred)) {
>  				skb_queue_head(&sk->sk_receive_queue, skb);
> +				skb = NULL;
>  				break;
>  			}
>  		} else {
> @@ -1967,6 +1968,7 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  		chunk = min_t(unsigned int, skb->len, size);
>  		if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
>  			skb_queue_head(&sk->sk_receive_queue, skb);
> +			skb = NULL;
>  			if (copied == 0)
>  				copied = -EFAULT;
>  			break;
> @@ -1984,13 +1986,14 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  			/* put the skb back if we didn't use it up.. */
>  			if (skb->len) {
>  				skb_queue_head(&sk->sk_receive_queue, skb);
> +				skb = NULL;
>  				break;
>  			}
>  
> -			consume_skb(skb);
> -
> -			if (siocb->scm->fp)
> +			if (UNIXCB(skb).pid || siocb->scm->fp)
>  				break;
> +			consume_skb(skb);
> +			skb = NULL;
>  		} else {
>  			/* It is questionable, see note in unix_dgram_recvmsg.
>  			 */
> @@ -1999,12 +2002,14 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  			/* put message back and return */
>  			skb_queue_head(&sk->sk_receive_queue, skb);
> +			skb = NULL;
>  			break;
>  		}
>  	} while (size);
>  
>  	mutex_unlock(&u->readlock);
>  	scm_recv(sock, msg, siocb->scm, flags);
> +	consume_skb(skb);
>  out:
>  	return copied ? : err;
>  }
> 
> 

Acked-by: Tim Chen <tim.c.chen@linux.intel.com>

^ permalink raw reply

* Re: [PATCH net-next v3] af_unix: Fix use-after-free crashes
From: Tim Chen @ 2011-09-09 10:41 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: sedat.dilek, Yan, Zheng, Yan, Zheng, netdev@vger.kernel.org,
	davem@davemloft.net, sfr@canb.auug.org.au, jirislaby@gmail.com,
	Shi, Alex, Valdis Kletnieks
In-Reply-To: <1315551100.5410.30.camel@edumazet-laptop>

On Fri, 2011-09-09 at 08:51 +0200, Eric Dumazet wrote:
> Le jeudi 08 septembre 2011 à 01:37 -0700, Tim Chen a écrit :
> > On Thu, 2011-09-08 at 15:21 +0200, Eric Dumazet wrote:
> > > Le jeudi 08 septembre 2011 à 11:59 +0200, Sedat Dilek a écrit :
> > > 
> > > > I have tested this fixup patch on i386.
> > > > Can we have a separate patch with corrected descriptive text?
> > > > 
> > > > Thanks to all involved people.
> > > 
> > > Here it is :
> > > 
> > > [PATCH net-next v3] af_unix: Fix use-after-free crashes
> > > 
> > > Commit 0856a30409 (Scm: Remove unnecessary pid & credential references
> > > in Unix socket's send and receive path) introduced an use-after-free
> > > bug.
> > > 
> > > We are allowed to steal the references to pid/cred only in the last skb
> > > sent from unix_stream_sendmsg(), because first skbs might be consumed by
> > > the receiver before we finish our sendmsg() call.
> > > 
> > > Remove scm_release() helper, since its cleaner to clear pid/cred fields
> > > in unix_scm_to_skb() when we steal them.
> > > 
> > > Based on prior patches from Yan Zheng and Tim Chen
> > > 
> > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > > Reported-by: Jiri Slaby <jirislaby@gmail.com>
> > > Tested-by: Sedat Dilek <sedat.dilek@googlemail.com>
> > > Tested-by: Valdis Kletnieks <Valdis.Kletnieks@vt.edu>
> > > ---
> > 
> > Thanks.
> > 
> > Acked-by: Tim Chen <tim.c.chen@linux.intel.com>
> > 
> 
> Now we have to fix a bug in unix_stream_recvmsg() as well.
> 

Thanks for your careful review to catch the bugs.  Wish I had thought
about the stream msg case more in my original patch.

> consume_skb() call actually releases pid/cred references, and we can use
> them after their eventual freeing.
> 
> Keep also in mind that receiver can provides a too short user buffer,
> and skb can be put back to head of sk_receive_queue
> 
> Tim, your 0856a304091b33 commit introduced a lot of bugs, I was right
> asking a revert.
> 
> If we revert your patch, my litle patch (af_unix: dont send
> SCM_CREDENTIALS by default) is enough to solve performance problems.
> 

But the regression where we do send SCM_CREDENTIALS is not addressed,
right?  I don't mind reverting my patch if there's a better solution.

Tim

^ permalink raw reply

* Re: [PATCH net v2 6/6] bnx2x: Fix ethtool advertisement
From: Ben Hutchings @ 2011-09-09 17:55 UTC (permalink / raw)
  To: Yaniv Rosner; +Cc: davem, eilong, netdev
In-Reply-To: <1315392491.27750.27.camel@lb-tlvb-dmitry>

On Wed, 2011-09-07 at 13:48 +0300, Yaniv Rosner wrote:
> Enable changing advertisement settings via ethtool.
> 
> Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
[...]

However, I suspect these fixes will not be considered important enough
for this release cycle, so you may need to be re-spin them for net-next.
David?

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

* Re: [PATCH v2 3/5] SUNRPC: make RPC service dependable on rpcbind clients creation
From: Trond Myklebust @ 2011-09-09 18:44 UTC (permalink / raw)
  To: Stanislav Kinsbursky
  Cc: Jeff Layton, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pavel Emelianov, neilb-l3A5Bk7waGM@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <4E6A41D4.6090001-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Fri, 2011-09-09 at 20:41 +0400, Stanislav Kinsbursky wrote: 
> 09.09.2011 18:07, Jeff Layton пишет:
> > On Fri, 09 Sep 2011 16:08:44 +0400
> > Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
> >
> >> Create rcbind clients or increase rpcbind users counter during RPC service
> >> creation and decrease this counter (and possibly destroy those clients) on RPC
> >> service destruction.
> >>
> >> Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> >>
> >> ---
> >>   include/linux/sunrpc/clnt.h |    2 ++
> >>   net/sunrpc/rpcb_clnt.c      |    2 +-
> >>   net/sunrpc/svc.c            |   13 +++++++++++--
> >>   3 files changed, 14 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> >> index db7bcaf..65a8115 100644
> >> --- a/include/linux/sunrpc/clnt.h
> >> +++ b/include/linux/sunrpc/clnt.h
> >> @@ -135,10 +135,12 @@ void		rpc_shutdown_client(struct rpc_clnt *);
> >>   void		rpc_release_client(struct rpc_clnt *);
> >>   void		rpc_task_release_client(struct rpc_task *);
> >>
> >> +int		rpcb_create_local(void);
> >>   int		rpcb_register(u32, u32, int, unsigned short);
> >>   int		rpcb_v4_register(const u32 program, const u32 version,
> >>   				 const struct sockaddr *address,
> >>   				 const char *netid);
> >> +void		rpcb_put_local(void);
> >>   void		rpcb_getport_async(struct rpc_task *);
> >>
> >>   void		rpc_call_start(struct rpc_task *);
> >> diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
> >> index b4cc0f1..437ec60 100644
> >> --- a/net/sunrpc/rpcb_clnt.c
> >> +++ b/net/sunrpc/rpcb_clnt.c
> >> @@ -318,7 +318,7 @@ out:
> >>    * Returns zero on success, otherwise a negative errno value
> >>    * is returned.
> >>    */
> >> -static int rpcb_create_local(void)
> >> +int rpcb_create_local(void)
> >>   {
> >>   	static DEFINE_MUTEX(rpcb_create_local_mutex);
> >>   	int result = 0;
> >> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> >> index 6a69a11..9095c0e 100644
> >> --- a/net/sunrpc/svc.c
> >> +++ b/net/sunrpc/svc.c
> >> @@ -367,8 +367,11 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> >>   	unsigned int xdrsize;
> >>   	unsigned int i;
> >>
> >> -	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> >> +	if (rpcb_create_local()<  0)
> >>   		return NULL;
> >> +
> >> +	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> >> +		goto out_err;
> >>   	serv->sv_name      = prog->pg_name;
> >>   	serv->sv_program   = prog;
> >>   	serv->sv_nrthreads = 1;
> >> @@ -403,7 +406,7 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> >>   			GFP_KERNEL);
> >>   	if (!serv->sv_pools) {
> >>   		kfree(serv);
> >> -		return NULL;
> >> +		goto out_err;
> >>   	}
> >>
> >>   	for (i = 0; i<  serv->sv_nrpools; i++) {
> >> @@ -423,6 +426,10 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> >>   	svc_unregister(serv);
> >>
> >>   	return serv;
> >> +
> >> +out_err:
> >> +	rpcb_put_local();
> >> +	return NULL;
> >>   }
> >>
> >>   struct svc_serv *
> >> @@ -491,6 +498,8 @@ svc_destroy(struct svc_serv *serv)
> >>   	svc_unregister(serv);
> >>   	kfree(serv->sv_pools);
> >>   	kfree(serv);
> >> +
> >> +	rpcb_put_local();
> >>   }
> >>   EXPORT_SYMBOL_GPL(svc_destroy);
> >>
> >>
> >
> > I don't get it -- what's the advantage of creating rpcbind clients in
> > __svc_create vs. the old way of creating them just before we plan to
> > use them?
> >
> 
> The main problem here is not in creation, but in destroying those clients.
> Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
> But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.

Could we perhaps set up a 'struct pernet_operations' to create a
destructor for them?

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
www.netapp.com

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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: [PATCH net v2 6/6] bnx2x: Fix ethtool advertisement
From: David Miller @ 2011-09-09 18:44 UTC (permalink / raw)
  To: bhutchings; +Cc: yanivr, eilong, netdev
In-Reply-To: <1315590931.2808.55.camel@bwh-desktop>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Fri, 09 Sep 2011 18:55:31 +0100

> On Wed, 2011-09-07 at 13:48 +0300, Yaniv Rosner wrote:
>> Enable changing advertisement settings via ethtool.
>> 
>> Signed-off-by: Yaniv Rosner <yanivr@broadcom.com>
>> Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
> [...]
> 
> However, I suspect these fixes will not be considered important enough
> for this release cycle, so you may need to be re-spin them for net-next.
> David?

Right, this stuff is not appropriate for the 'net' GIT tree and must
go into 'net-next'.

^ permalink raw reply

* Re: [PATCH v2 3/5] SUNRPC: make RPC service dependable on rpcbind clients creation
From: Jeff Layton @ 2011-09-09 19:01 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: Stanislav Kinsbursky,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pavel Emelianov, neilb-l3A5Bk7waGM@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <1315593874.17611.19.camel-SyLVLa/KEI9HwK5hSS5vWB2eb7JE58TQ@public.gmane.org>

On Fri, 09 Sep 2011 14:44:34 -0400
Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> wrote:

> On Fri, 2011-09-09 at 20:41 +0400, Stanislav Kinsbursky wrote: 
> > 09.09.2011 18:07, Jeff Layton пишет:
> > > On Fri, 09 Sep 2011 16:08:44 +0400
> > > Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
> > >
> > >> Create rcbind clients or increase rpcbind users counter during RPC service
> > >> creation and decrease this counter (and possibly destroy those clients) on RPC
> > >> service destruction.
> > >>
> > >> Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> > >>
> > >> ---
> > >>   include/linux/sunrpc/clnt.h |    2 ++
> > >>   net/sunrpc/rpcb_clnt.c      |    2 +-
> > >>   net/sunrpc/svc.c            |   13 +++++++++++--
> > >>   3 files changed, 14 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> > >> index db7bcaf..65a8115 100644
> > >> --- a/include/linux/sunrpc/clnt.h
> > >> +++ b/include/linux/sunrpc/clnt.h
> > >> @@ -135,10 +135,12 @@ void		rpc_shutdown_client(struct rpc_clnt *);
> > >>   void		rpc_release_client(struct rpc_clnt *);
> > >>   void		rpc_task_release_client(struct rpc_task *);
> > >>
> > >> +int		rpcb_create_local(void);
> > >>   int		rpcb_register(u32, u32, int, unsigned short);
> > >>   int		rpcb_v4_register(const u32 program, const u32 version,
> > >>   				 const struct sockaddr *address,
> > >>   				 const char *netid);
> > >> +void		rpcb_put_local(void);
> > >>   void		rpcb_getport_async(struct rpc_task *);
> > >>
> > >>   void		rpc_call_start(struct rpc_task *);
> > >> diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
> > >> index b4cc0f1..437ec60 100644
> > >> --- a/net/sunrpc/rpcb_clnt.c
> > >> +++ b/net/sunrpc/rpcb_clnt.c
> > >> @@ -318,7 +318,7 @@ out:
> > >>    * Returns zero on success, otherwise a negative errno value
> > >>    * is returned.
> > >>    */
> > >> -static int rpcb_create_local(void)
> > >> +int rpcb_create_local(void)
> > >>   {
> > >>   	static DEFINE_MUTEX(rpcb_create_local_mutex);
> > >>   	int result = 0;
> > >> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> > >> index 6a69a11..9095c0e 100644
> > >> --- a/net/sunrpc/svc.c
> > >> +++ b/net/sunrpc/svc.c
> > >> @@ -367,8 +367,11 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> > >>   	unsigned int xdrsize;
> > >>   	unsigned int i;
> > >>
> > >> -	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> > >> +	if (rpcb_create_local()<  0)
> > >>   		return NULL;
> > >> +
> > >> +	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> > >> +		goto out_err;
> > >>   	serv->sv_name      = prog->pg_name;
> > >>   	serv->sv_program   = prog;
> > >>   	serv->sv_nrthreads = 1;
> > >> @@ -403,7 +406,7 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> > >>   			GFP_KERNEL);
> > >>   	if (!serv->sv_pools) {
> > >>   		kfree(serv);
> > >> -		return NULL;
> > >> +		goto out_err;
> > >>   	}
> > >>
> > >>   	for (i = 0; i<  serv->sv_nrpools; i++) {
> > >> @@ -423,6 +426,10 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> > >>   	svc_unregister(serv);
> > >>
> > >>   	return serv;
> > >> +
> > >> +out_err:
> > >> +	rpcb_put_local();
> > >> +	return NULL;
> > >>   }
> > >>
> > >>   struct svc_serv *
> > >> @@ -491,6 +498,8 @@ svc_destroy(struct svc_serv *serv)
> > >>   	svc_unregister(serv);
> > >>   	kfree(serv->sv_pools);
> > >>   	kfree(serv);
> > >> +
> > >> +	rpcb_put_local();
> > >>   }
> > >>   EXPORT_SYMBOL_GPL(svc_destroy);
> > >>
> > >>
> > >
> > > I don't get it -- what's the advantage of creating rpcbind clients in
> > > __svc_create vs. the old way of creating them just before we plan to
> > > use them?
> > >
> > 
> > The main problem here is not in creation, but in destroying those clients.
> > Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
> > But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
> 
> Could we perhaps set up a 'struct pernet_operations' to create a
> destructor for them?
> 

An even easier idea might be to just not take a reference to the
rpcbind client for svc_programs that have vs_hidden set on every
version.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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: [PATCH] RDSRDMA: Fix cleanup of rds_iw_mr_pool
From: Steve Wise @ 2011-09-09 19:01 UTC (permalink / raw)
  To: Davem; +Cc: Venkat Venkatsubra, Netdev, Rds-Devel, Jonathan Lallinger
In-Reply-To: <67e08fa0-c2e1-44f8-ab8b-45e4bfdc9264@default>

On 09/09/2011 09:37 AM, Venkat Venkatsubra wrote:
> On 09/08/2011 01:04 PM, Jonathan Lallinger wrote:
>> In the rds_iw_mr_pool struct the free_pinned field keeps track of memory pinned
>> by free MRs. While this field is incremented properly upon allocation, it is never
>> decremented upon unmapping. This would cause the rds_rdma module to crash the
>> kernel upon unloading, by triggering the BUG_ON in the rds_iw_destroy_mr_pool
>> function.
>>
>> This change keeps track of the MRs that become unpinned, so that free_pinned
>> can be decremented appropriately.
>>
>> Signed-off-by: Jonathan Lallinger<jonathan@ogc.us>
>> Signed-off-by: Steve Wise<swise@ogc.us>
> Signed-off-by: Venkat Venkatsubra<venkat.x.venkatsubra@oracle.com>
>
> Venkat
>

Hey Dave,

Is this sufficient for you to pull/merge the patch upstream?  Or should Venkat, the new RDS maintainer, do something 
else?  Like maybe coalesce patches into a git repos and ask you to pull them in?  I'm not sure what process you had with 
the old RDS maintainer.

There's another patch too from Jonathan Lallinger that fixes an RDS crash titled:

[PATCH] RDSRDMA: Fix to PAGE_MASK interpretation

Both have been signed off by Venkat and reviewed by me.


Thanks,

Steve.

^ permalink raw reply

* Re: [PATCH] RDSRDMA: Fix cleanup of rds_iw_mr_pool
From: David Miller @ 2011-09-09 19:05 UTC (permalink / raw)
  To: swise; +Cc: venkat.x.venkatsubra, netdev, rds-devel, jonathan
In-Reply-To: <4E6A62A1.1010702@opengridcomputing.com>

From: Steve Wise <swise@opengridcomputing.com>
Date: Fri, 09 Sep 2011 14:01:53 -0500

> Is this sufficient for you to pull/merge the patch upstream?

It's all queued up in patchwork, so I don't see why not.

^ permalink raw reply

* Re: Using gretap to tunnel layer 2 traffic
From: Ben Hutchings @ 2011-09-09 19:19 UTC (permalink / raw)
  To: John H; +Cc: netdev@vger.kernel.org
In-Reply-To: <1315589104.22676.YahooMailNeo@web45104.mail.sp1.yahoo.com>

On Fri, 2011-09-09 at 10:25 -0700, John H wrote:
> I am attempting to tunnel Layer 2 traffic through a gretap device, 
> while encrypting the GRE tunnel with IPsec. My test environment is as follows:
> 
> 10.0.1.1                                          10.0.1.2
> client_a <--> server_left <==> server_right <---> client_b
>                       gretap/IPsec
>                       
> 
> On the servers, I have two VLANs per server, corresponding to the unencrypted
> and encrypted interfaces.  On each server, the unencrypted VLAN is 
> bridged with the gretap device.  All VLANs and physical devices have MTUs of 
> 1500.  The gretap device has a resultant MTU of 1462, thereby causing the 
> bridge device to have an MTU of 1462.
> 
> Traffic for the most part works as it is expected to behave. However, 
> packets are dropped when client_a sends an ICMP packet to client_b which 
> has an MTU less than client_a's device MTU but larger than server_left's 
> MTU. I suspect other protocols would behave similarly (silently dropping
> packets).  I an running "ping -c 1 -s 1450 10.0.1.2" on client_a, this results
> in an ICMP packet being sent with an MTU of 1478.
> 
> An MTU of 1478 is larger than the bridge device's MTU of 1462, causing the 
> packet to be silently discarded per net/bridge/br_forward.c 
> in function br_dev_queue_push_xmit:
>
> int br_dev_queue_push_xmit(struct sk_buff *skb)
> {
>     /* drop mtu oversized packets except gso */
>     if (packet_length(skb) > skb->dev->mtu && !skb_is_gso(skb))
>         kfree_skb(skb);
>     else {
>     ....
>     
> If the gretap device supported GSO, I suspect that this would not be a
> problem. (ethtool -k gretapLeftRight states that GSO/GRO/LRO is not 
> supported)

GRO+GSO may generally be used when forwarding TCP packets.  But aside
from that, none of these have any effect on forwarded packets.

> Function br_dev_queue_push_xmit eventually calls ipgre_tunnel_xmit, if the 
> packet is not dropped.
> 
> I would think that br_dev_queue_push_xmit should call ipgre_tunnel_xmit
> regardless of the device MTU and ipgre_tunnel_xmit would handle packet
> fragmentation internally, but it never has the chance.
> 
> I have tried allowing all packets through br_dev_queue_push_xmit
> and explicitly setting the Don't Frament field to 0 in ipgre_tunnel_xmit,
> but this didn't solve problem.
> 
> Given that this would be tunneling Layer 2 traffic, it wouldn't make sense
> to send an ICMP_FRAG_NEEDED response from the bridge.

Right.

> The real question is, however, why is any client able to send a single ICMP
> packet with size 1478 bytes when one of the hops along the way only 
> supports 1462 bytes per its MTU? Shouldn't this have been negotiated 
> beforehand?

The DHCP and/or route advertisement daemons should tell hosts what the
correct MTU is for the subnet they are 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.

^ permalink raw reply

* Re: [PATCH v2 3/5] SUNRPC: make RPC service dependable on rpcbind clients creation
From: Trond Myklebust @ 2011-09-09 19:25 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Stanislav Kinsbursky,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Pavel Emelianov, neilb-l3A5Bk7waGM@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <20110909150104.5a83c60d-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>

On Fri, 2011-09-09 at 15:01 -0400, Jeff Layton wrote: 
> On Fri, 09 Sep 2011 14:44:34 -0400
> Trond Myklebust <Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > On Fri, 2011-09-09 at 20:41 +0400, Stanislav Kinsbursky wrote: 
> > > 09.09.2011 18:07, Jeff Layton пишет:
> > > > On Fri, 09 Sep 2011 16:08:44 +0400
> > > > Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>  wrote:
> > > >
> > > >> Create rcbind clients or increase rpcbind users counter during RPC service
> > > >> creation and decrease this counter (and possibly destroy those clients) on RPC
> > > >> service destruction.
> > > >>
> > > >> Signed-off-by: Stanislav Kinsbursky<skinsbursky-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> > > >>
> > > >> ---
> > > >>   include/linux/sunrpc/clnt.h |    2 ++
> > > >>   net/sunrpc/rpcb_clnt.c      |    2 +-
> > > >>   net/sunrpc/svc.c            |   13 +++++++++++--
> > > >>   3 files changed, 14 insertions(+), 3 deletions(-)
> > > >>
> > > >> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> > > >> index db7bcaf..65a8115 100644
> > > >> --- a/include/linux/sunrpc/clnt.h
> > > >> +++ b/include/linux/sunrpc/clnt.h
> > > >> @@ -135,10 +135,12 @@ void		rpc_shutdown_client(struct rpc_clnt *);
> > > >>   void		rpc_release_client(struct rpc_clnt *);
> > > >>   void		rpc_task_release_client(struct rpc_task *);
> > > >>
> > > >> +int		rpcb_create_local(void);
> > > >>   int		rpcb_register(u32, u32, int, unsigned short);
> > > >>   int		rpcb_v4_register(const u32 program, const u32 version,
> > > >>   				 const struct sockaddr *address,
> > > >>   				 const char *netid);
> > > >> +void		rpcb_put_local(void);
> > > >>   void		rpcb_getport_async(struct rpc_task *);
> > > >>
> > > >>   void		rpc_call_start(struct rpc_task *);
> > > >> diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
> > > >> index b4cc0f1..437ec60 100644
> > > >> --- a/net/sunrpc/rpcb_clnt.c
> > > >> +++ b/net/sunrpc/rpcb_clnt.c
> > > >> @@ -318,7 +318,7 @@ out:
> > > >>    * Returns zero on success, otherwise a negative errno value
> > > >>    * is returned.
> > > >>    */
> > > >> -static int rpcb_create_local(void)
> > > >> +int rpcb_create_local(void)
> > > >>   {
> > > >>   	static DEFINE_MUTEX(rpcb_create_local_mutex);
> > > >>   	int result = 0;
> > > >> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> > > >> index 6a69a11..9095c0e 100644
> > > >> --- a/net/sunrpc/svc.c
> > > >> +++ b/net/sunrpc/svc.c
> > > >> @@ -367,8 +367,11 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> > > >>   	unsigned int xdrsize;
> > > >>   	unsigned int i;
> > > >>
> > > >> -	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> > > >> +	if (rpcb_create_local()<  0)
> > > >>   		return NULL;
> > > >> +
> > > >> +	if (!(serv = kzalloc(sizeof(*serv), GFP_KERNEL)))
> > > >> +		goto out_err;
> > > >>   	serv->sv_name      = prog->pg_name;
> > > >>   	serv->sv_program   = prog;
> > > >>   	serv->sv_nrthreads = 1;
> > > >> @@ -403,7 +406,7 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> > > >>   			GFP_KERNEL);
> > > >>   	if (!serv->sv_pools) {
> > > >>   		kfree(serv);
> > > >> -		return NULL;
> > > >> +		goto out_err;
> > > >>   	}
> > > >>
> > > >>   	for (i = 0; i<  serv->sv_nrpools; i++) {
> > > >> @@ -423,6 +426,10 @@ __svc_create(struct svc_program *prog, unsigned int bufsize, int npools,
> > > >>   	svc_unregister(serv);
> > > >>
> > > >>   	return serv;
> > > >> +
> > > >> +out_err:
> > > >> +	rpcb_put_local();
> > > >> +	return NULL;
> > > >>   }
> > > >>
> > > >>   struct svc_serv *
> > > >> @@ -491,6 +498,8 @@ svc_destroy(struct svc_serv *serv)
> > > >>   	svc_unregister(serv);
> > > >>   	kfree(serv->sv_pools);
> > > >>   	kfree(serv);
> > > >> +
> > > >> +	rpcb_put_local();
> > > >>   }
> > > >>   EXPORT_SYMBOL_GPL(svc_destroy);
> > > >>
> > > >>
> > > >
> > > > I don't get it -- what's the advantage of creating rpcbind clients in
> > > > __svc_create vs. the old way of creating them just before we plan to
> > > > use them?
> > > >
> > > 
> > > The main problem here is not in creation, but in destroying those clients.
> > > Now rpcbind clients are created during rpcb_register(). I.e. once per every family, program version and so on.
> > > But can be unregistered for all protocol families by one call. So it's impossible to put reference counting for those clients in the place, where they are created now.
> > 
> > Could we perhaps set up a 'struct pernet_operations' to create a
> > destructor for them?
> > 
> 
> An even easier idea might be to just not take a reference to the
> rpcbind client for svc_programs that have vs_hidden set on every
> version.

Isn't the problem that Stanislav is trying to solve that we need to be
able to register and unregister RPC services to the correct rpcbind
server, depending on which net namespace we are in?

My understanding is that the current code will register everything to
whatever rpcbind server is running in the init net namespace because
that's what rpcb_create_local() uses.

My suggestion is to use a struct pernet_operations to detect when a net
namespace is being created or destroyed, so that the rpcbind client code
knows when to create or destroy a connection to the server that is
running in that namespace.

Cheers
  Trond
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
www.netapp.com

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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


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