Netdev List
 help / color / mirror / Atom feed
* Re: [*v3 PATCH 01/22] IPVS: netns, add basic init per netns.
From: Hans Schillstrom @ 2010-12-31 15:36 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: horms, ja, daniel.lezcano, wensong, lvs-devel, netdev,
	netfilter-devel, Hans Schillstrom
In-Reply-To: <alpine.LNX.2.01.1012310055050.15397@obet.zrqbmnf.qr>


On Friday, December 31, 2010 00:58:35 Jan Engelhardt wrote:
> 
> On Thursday 2010-12-30 11:50, hans@schillstrom.com wrote:
> >+++ b/include/net/ip_vs.h
> >@@ -28,6 +28,15 @@
> > #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
> > #include <net/netfilter/nf_conntrack.h>
> > #endif
> >+#include <net/net_namespace.h>		/* Netw namespace */
> >+
> >+/*
> >+ * Generic access of ipvs struct
> >+ */
> >+static inline struct netns_ipvs * net_ipvs(struct net* net)
> 
> Balancing the '*' between the ' ' would be nice... I know it's hard at this
> time of the year ;-)
> 
I'll send a new patch set and run checkpatch,
but that will be after new years eve :-)

> >diff --git a/include/net/netns/ip_vs.h b/include/net/netns/ip_vs.h
> >new file mode 100644
> >index 0000000..9068d95
> >--- /dev/null
> >+++ b/include/net/netns/ip_vs.h
> >@@ -0,0 +1,26 @@
> >+/*
> >+ * ip_vs.h
> >+ *
> >+ *  Created on: Nov 23, 2010
> >+ *  Author: hans
> >+ */
> 
> Filenames, creation dates, author names (when they don't serve a copyright
> notice), CVS $Id$ tags, etc. don't belong into files. The git log exists for
> that very purpose.
> 
Oops that's an eclipse feature, that should be removed.

> >+struct netns_ipvs {
> >+	int			inc;		/* Incarnation */
> >+};
> 
> Just my thoughts: Incarnation - haven't heard that in a while. ("inc"
> is also used as an abbreviation for increment, so ~ ~). Other places
> use "generation" as a term (such as the VFS on inode generation). [I
> also get the feeling that (re)"incarnation" may require a death, so
> it is different from "generation" after all -]

:-))

> 
> >+++ b/net/netfilter/ipvs/ip_vs_core.c
> >@@ -1813,6 +1820,44 @@ static struct nf_hook_ops ip_vs_ops[] __read_mostly = {
> > #endif
> > };
> >
> >+/*
> >+ *	Initialize IP Virtual Server netns mem.
> >+ */
> >+static int __net_init  __ip_vs_init(struct net *net)
> >+{
> >+	struct netns_ipvs *ipvs;
> >+
> >+	if (!net_eq(net, &init_net)) {
> >+		pr_err("The final patch for enabling netns is missing\n");
> >+		return -EPERM;
> >+	}
> >+	ipvs = (struct netns_ipvs *)net_generic(net, ip_vs_net_id);
> 
> Pointless cast is pointless.

Well, that is a reminder from a backport...

> 
> >index c1c167a..ea390f8 100644
> >--- a/net/netfilter/ipvs/ip_vs_sync.c
> >+++ b/net/netfilter/ipvs/ip_vs_sync.c
> >@@ -1639,3 +1639,31 @@ int stop_sync_thread(int state)
> >
> > 	return 0;
> > }
> >+
> >+/*
> >+ * Initialize data struct for each netns
> >+ */
> >+static int __net_init __ip_vs_sync_init(struct net *net)
> >+{
> >+	return 0;
> >+}
> >+
> >+static void __ip_vs_sync_cleanup(struct net *net)
> >+{
> >+	return;
> >+}
> 
> The trailing return; in functions returning nothing can be (and
> generally, is) omitted.
> 
Oops

Thanks alot 

/Hans


^ permalink raw reply

* Re: [PATCH 07/15]drivers:net:wireless:iwlwifi Typo change diable to disable.
From: Larry Finger @ 2010-12-31 15:33 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: trivial, linux-m68k, linux-kernel, netdev, ivtv-devel,
	linux-media, linux-wireless, linux-scsi, spi-devel-general, devel,
	linux-usb
In-Reply-To: <1293750484-1161-7-git-send-email-justinmattock@gmail.com>

On 12/30/2010 05:07 PM, Justin P. Mattock wrote:
> The below patch fixes a typo "diable" to "disable". Please let me know if this 
> is correct or not.
> 
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> 

ACKed-by: Larry Finger <Larry.Finger@lwfinger.net>


> ---
>  drivers/net/wireless/iwlwifi/iwl-agn-ict.c |    2 +-
>  drivers/net/wireless/iwlwifi/iwl-agn.c     |    4 ++--
>  drivers/net/wireless/iwlwifi/iwl-core.c    |    2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-ict.c b/drivers/net/wireless/iwlwifi/iwl-agn-ict.c
> index a5dbfea..b5cb3be 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-ict.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-ict.c
> @@ -197,7 +197,7 @@ static irqreturn_t iwl_isr(int irq, void *data)
>  
>   none:
>  	/* re-enable interrupts here since we don't have anything to service. */
> -	/* only Re-enable if diabled by irq  and no schedules tasklet. */
> +	/* only Re-enable if disabled by irq  and no schedules tasklet. */
>  	if (test_bit(STATUS_INT_ENABLED, &priv->status) && !priv->_agn.inta)
>  		iwl_enable_interrupts(priv);
>  
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index c2636a7..9b912c0 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -1316,7 +1316,7 @@ static void iwl_irq_tasklet_legacy(struct iwl_priv *priv)
>  	}
>  
>  	/* Re-enable all interrupts */
> -	/* only Re-enable if diabled by irq */
> +	/* only Re-enable if disabled by irq */
>  	if (test_bit(STATUS_INT_ENABLED, &priv->status))
>  		iwl_enable_interrupts(priv);
>  
> @@ -1530,7 +1530,7 @@ static void iwl_irq_tasklet(struct iwl_priv *priv)
>  	}
>  
>  	/* Re-enable all interrupts */
> -	/* only Re-enable if diabled by irq */
> +	/* only Re-enable if disabled by irq */
>  	if (test_bit(STATUS_INT_ENABLED, &priv->status))
>  		iwl_enable_interrupts(priv);
>  }
> diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
> index 25fb391..8700ab3 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-core.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-core.c
> @@ -1304,7 +1304,7 @@ irqreturn_t iwl_isr_legacy(int irq, void *data)
>  
>   none:
>  	/* re-enable interrupts here since we don't have anything to service. */
> -	/* only Re-enable if diabled by irq */
> +	/* only Re-enable if disabled by irq */
>  	if (test_bit(STATUS_INT_ENABLED, &priv->status))
>  		iwl_enable_interrupts(priv);
>  	spin_unlock_irqrestore(&priv->lock, flags);


^ permalink raw reply

* Re: [PATCH 03/15]drivers:staging:rtl8187se:r8180_hw.h Typo change diable to disable.
From: Larry Finger @ 2010-12-31 15:32 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: trivial, linux-m68k, linux-kernel, netdev, ivtv-devel,
	linux-media, linux-wireless, linux-scsi, spi-devel-general, devel,
	linux-usb
In-Reply-To: <1293750484-1161-3-git-send-email-justinmattock@gmail.com>

On 12/30/2010 05:07 PM, Justin P. Mattock wrote:
> The below patch fixes a typo "diable" to "disable". Please let me know if this 
> is correct or not.
> 
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
> 
> ---

ACKed-by: Larry Finger <Larry.Finger@lwfinger.net>

>  drivers/staging/rtl8187se/r8180_hw.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/rtl8187se/r8180_hw.h b/drivers/staging/rtl8187se/r8180_hw.h
> index 3fca144..2911d40 100644
> --- a/drivers/staging/rtl8187se/r8180_hw.h
> +++ b/drivers/staging/rtl8187se/r8180_hw.h
> @@ -554,7 +554,7 @@
>  /* by amy for power save		*/
>  /* by amy for antenna			*/
>  #define EEPROM_SW_REVD_OFFSET 0x3f
> -/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other values are diable.					*/
> +/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other values are disabled.					*/
>  #define EEPROM_SW_AD_MASK			0x0300
>  #define EEPROM_SW_AD_ENABLE			0x0100
>  


^ permalink raw reply

* Re: [PATCH] net: eepro testing positive EBUSY return by request_irq()?
From: Ben Hutchings @ 2010-12-31 15:27 UTC (permalink / raw)
  To: roel kluin; +Cc: davem, netdev, Andrew Morton, LKML
In-Reply-To: <4D1DECFC.8020701@gmail.com>

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

On Fri, 2010-12-31 at 15:47 +0100, roel kluin wrote:
> Fix -EBUSY test for request_irq().
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
>  drivers/net/eepro.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> I just found this in the code, no bug was observed. Is this patch needed? the test
> for an -EBUSY return by request_irq() occurs much more frequently in kernel code.
> 
> diff --git a/drivers/net/eepro.c b/drivers/net/eepro.c
> index 7c82631..47cfecb 100644
> --- a/drivers/net/eepro.c
> +++ b/drivers/net/eepro.c
> @@ -920,7 +920,7 @@ static int	eepro_grab_irq(struct net_device *dev)
>  
>  		eepro_sw2bank0(ioaddr); /* Switch back to Bank 0 */
>  
> -		if (request_irq (*irqp, NULL, IRQF_SHARED, "bogus", dev) != EBUSY) {
> +		if (request_irq (*irqp, NULL, IRQF_SHARED, "bogus", dev) != -EBUSY) {
>  			unsigned long irq_mask;
>  			/* Twinkle the interrupt, and check if it's seen */
>  			irq_mask = probe_irq_on();

This condition is completely bogus - request_irq() with a NULL handler
now returns -EINVAL before even checking whether the IRQ is in use.  The
code should be fixed along the lines of what I did for 3c503 in commit
b0cf4dfb7cd21556efd9a6a67edcba0840b4d98d.

The e2100 and hp net drivers have the same bug.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [PATCH 02/15]drivers:spi:dw_spi.c Typo change diable to disable.
From: James Bottomley @ 2010-12-31 15:06 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: Dan Carpenter, Grant Likely, trivial-DgEjT+Ai2ygdnm+yROfE0A,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	ivtv-devel-jGorlIydJmRM656bX5wj8A,
	linux-m68k-cunTk1MwBs8S/qaLPR03pWD2FQJk+8+b,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4D1DE616.7010105-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Fri, 2010-12-31 at 06:17 -0800, Justin P. Mattock wrote:
> On 12/31/2010 01:11 AM, Dan Carpenter wrote:
> > On Thu, Dec 30, 2010 at 10:52:30PM -0800, Justin P. Mattock wrote:
> >> On 12/30/2010 10:45 PM, Grant Likely wrote:
> >>> On Thu, Dec 30, 2010 at 03:07:51PM -0800, Justin P. Mattock wrote:
> >>>> The below patch fixes a typo "diable" to "disable". Please let me know if this
> >>>> is correct or not.
> >>>>
> >>>> Signed-off-by: Justin P. Mattock<justinmattock-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>>
> >>> applied, thanks.
> >>>
> >>> g.
> >>
> >> ahh.. thanks.. just cleared up the left out diabled that I had
> >> thought I forgotten(ended up separating comments and code and
> >> forgot)
> >
> > This is really just defensiveness and random grumbling and grumpiness on
> > my part, but one reason I may have missed the first patch is because
> > your subject lines are crap.
> >
> > Wrong:  [PATCH 02/15]drivers:spi:dw_spi.c Typo change diable to disable.
> >
> > Right:  [PATCH 02/15] spi/dw_spi: Typo change diable to disable
> >
> > regards,
> > dan carpenter
> >
> 
> alright.. so having the backlash is alright for the subject
> 
> Thanks for the pointer on this..

There is actually no specific form.  Most of us edit this part of the
subject line anyway to conform to whatever (nonuniform) conventions we
use.  I just use <component>: with no scsi or drivers prefix because the
git tree is tagged [SCSI]; others are different.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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] net: eepro testing positive EBUSY return by request_irq()?
From: roel kluin @ 2010-12-31 14:47 UTC (permalink / raw)
  To: davem, netdev, Andrew Morton, LKML

Fix -EBUSY test for request_irq().

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
 drivers/net/eepro.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

I just found this in the code, no bug was observed. Is this patch needed? the test
for an -EBUSY return by request_irq() occurs much more frequently in kernel code.

diff --git a/drivers/net/eepro.c b/drivers/net/eepro.c
index 7c82631..47cfecb 100644
--- a/drivers/net/eepro.c
+++ b/drivers/net/eepro.c
@@ -920,7 +920,7 @@ static int	eepro_grab_irq(struct net_device *dev)
 
 		eepro_sw2bank0(ioaddr); /* Switch back to Bank 0 */
 
-		if (request_irq (*irqp, NULL, IRQF_SHARED, "bogus", dev) != EBUSY) {
+		if (request_irq (*irqp, NULL, IRQF_SHARED, "bogus", dev) != -EBUSY) {
 			unsigned long irq_mask;
 			/* Twinkle the interrupt, and check if it's seen */
 			irq_mask = probe_irq_on();

^ permalink raw reply related

* Re: [PATCH 02/15]drivers:spi:dw_spi.c Typo change diable to disable.
From: Justin P. Mattock @ 2010-12-31 14:17 UTC (permalink / raw)
  To: Dan Carpenter, Grant Likely, trivial, devel, linux-scsi, netdev,
	li
In-Reply-To: <20101231091136.GC1886@bicker>

On 12/31/2010 01:11 AM, Dan Carpenter wrote:
> On Thu, Dec 30, 2010 at 10:52:30PM -0800, Justin P. Mattock wrote:
>> On 12/30/2010 10:45 PM, Grant Likely wrote:
>>> On Thu, Dec 30, 2010 at 03:07:51PM -0800, Justin P. Mattock wrote:
>>>> The below patch fixes a typo "diable" to "disable". Please let me know if this
>>>> is correct or not.
>>>>
>>>> Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
>>>
>>> applied, thanks.
>>>
>>> g.
>>
>> ahh.. thanks.. just cleared up the left out diabled that I had
>> thought I forgotten(ended up separating comments and code and
>> forgot)
>
> This is really just defensiveness and random grumbling and grumpiness on
> my part, but one reason I may have missed the first patch is because
> your subject lines are crap.
>
> Wrong:  [PATCH 02/15]drivers:spi:dw_spi.c Typo change diable to disable.
>
> Right:  [PATCH 02/15] spi/dw_spi: Typo change diable to disable
>
> regards,
> dan carpenter
>

alright.. so having the backlash is alright for the subject

Thanks for the pointer on this..

Justin P. Mattock

^ permalink raw reply

* Re: [PATCH 14/15]include:media:davinci:vpss.h Typo change diable to disable.
From: Justin P. Mattock @ 2010-12-31 14:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: trivial, linux-m68k, linux-kernel, netdev, ivtv-devel,
	linux-media, linux-wireless, linux-scsi, spi-devel-general, devel,
	linux-usb
In-Reply-To: <4D1DAFF5.3090108@gmail.com>

On 12/31/2010 02:27 AM, Mauro Carvalho Chehab wrote:
> Em 30-12-2010 21:08, Justin P. Mattock escreveu:
>> The below patch fixes a typo "diable" to "disable". Please let me know if this
>> is correct or not.
>>
>> Signed-off-by: Justin P. Mattock<justinmattock@gmail.com>
> Acked-by: Mauro Carvalho Chehab<mchehab@redhat.com>
>
> PS.: Next time, please c/c linux-media ONLY on patches related to media
> drivers (/drivers/video and the corresponding include files). Having to
> dig into a series of 15 patches to just actually look on 3 patches
> is not nice.
>

alright...

>>
>> ---
>>   include/media/davinci/vpss.h |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/media/davinci/vpss.h b/include/media/davinci/vpss.h
>> index c59cc02..b586495 100644
>> --- a/include/media/davinci/vpss.h
>> +++ b/include/media/davinci/vpss.h
>> @@ -44,7 +44,7 @@ struct vpss_pg_frame_size {
>>   	short pplen;
>>   };
>>
>> -/* Used for enable/diable VPSS Clock */
>> +/* Used for enable/disable VPSS Clock */
>>   enum vpss_clock_sel {
>>   	/* DM355/DM365 */
>>   	VPSS_CCDC_CLOCK,
>
>

Justin P. Mattock

^ permalink raw reply

* Re: [PATCH v2 00/12] make rpc_pipefs be mountable multiple time
From: Kirill A. Shutemov @ 2010-12-31 13:03 UTC (permalink / raw)
  To: Rob Landley
  Cc: Kirill A. Shutemov, Rob Landley, Trond Myklebust, J. Bruce Fields,
	Neil Brown, Pavel Emelyanov, linux-nfs, David S. Miller, netdev,
	linux-kernel
In-Reply-To: <4D1C809B.30405@parallels.com>

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

On Thu, Dec 30, 2010 at 06:52:43AM -0600, Rob Landley wrote:
> On 12/30/2010 05:45 AM, Kirill A. Shutemov wrote:
> > Currently, there is no association between rpc_pipefs and mount namespace,
> 
> There is in that the root context doesn't need to have this mounted, and 
> new namespaces do.  So there's an existing association between a LACK of 
> a namespace and a different default behavior.
>
> My understanding (correct me if I'm wrong) is that the historical 
> behavior is that there's only one, and it doesn't actually live anywhere 
> in the filesystem tree.  You're adding a special location.  I'm 
> wondering if there's any way for that location not to be special.

/var/lib/net/rpc_pipefs is default path where userspace part of NFS stack
(gssd, idmapd) want to see rpc_pipefs

> > so I don't see simple way to restrict number of rpc_pipefs per mount
> > namespace. Associating mount namespace with rpc_pipefs is not a good idea,
> > I think.
> 
> I'm talking about associating a default rpc_pipefs instance with a 
> namespace, which it seems to me you're already doing by emulating the 
> legacy behavior.  Before you CLONE_NEWNS you get a magic default mount 
> that doesn't exist in the tree.  After you CLONE_NEWNS you get something 
> like -EINVAL unless you supply your own default.

Root namespace is special. In case of nfsroot you need rpc_pipefs before
root available.

> (I'm actually not sure 
> why new namespaces don't fall back to the magic global one...)

It breaks isolation. Container should not use host's rpc_pipefs without
host's permission.
 
> I'm suggesting that if the user doesn't specify -o rpcmount then the 
> default could be the first rpc_pipefs mount visible to the current 
> process context, rather than a specific path.  Logic to do that exists 
> in the proc/self/mounts code (which I'm reading through now...).

static int check_rpc_pipefs(struct vfsmount *mnt, void *arg)
{
        struct vfsmount **rpcmount = arg;
        struct path path = {
                .mnt = mnt,
                .dentry = mnt->mnt_root,
        };

        if (!mnt->mnt_sb)
                return 0;
        if (mnt->mnt_sb->s_magic != RPCAUTH_GSSMAGIC)
                return 0;

        if (!path_is_under(&path, &current->fs->root))
                return 0;

        *rpcmount = mntget(mnt);
        return 1;
}

struct vfsmount *get_rpc_pipefs(const char *p)
{
        int error;
        struct vfsmount *rpcmount = ERR_PTR(-EINVAL);
        struct path path;

        if (!p) {
                iterate_mounts(check_rpc_pipefs, &rpcmount,
                                current->nsproxy->mnt_ns->root);

                if (IS_ERR(rpcmount) && (current->nsproxy->mnt_ns ==
                                        init_task.nsproxy->mnt_ns))
                        return mntget(init_rpc_pipefs);

                return rpcmount;
        }

        error = kern_path(p, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &path);
        if (error)
                return ERR_PTR(error);

        check_rpc_pipefs(path.mnt, &rpcmount);
        path_put(&path);

        return rpcmount;
}
EXPORT_SYMBOL_GPL(get_rpc_pipefs);

Something like this? Patch to replace patch #10 attached.

-- 
 Kirill A. Shutemov

[-- Attachment #2: sunrpc-introduce-get_rpc_pipefs.patch --]
[-- Type: text/plain, Size: 2466 bytes --]

>From 36bdb502360461a8426821a37728aef3a3b8c738 Mon Sep 17 00:00:00 2001
From: Kirill A. Shutemov <kas@openvz.org>
Date: Mon, 20 Dec 2010 04:03:52 +0200
Subject: [PATCH] sunrpc: introduce get_rpc_pipefs()

Get rpc_pipefs mount point by path.

Signed-off-by: Kirill A. Shutemov <kas@openvz.org>
---
 include/linux/sunrpc/rpc_pipe_fs.h |    2 +
 net/sunrpc/rpc_pipe.c              |   51 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/include/linux/sunrpc/rpc_pipe_fs.h b/include/linux/sunrpc/rpc_pipe_fs.h
index b09bfa5..922057c 100644
--- a/include/linux/sunrpc/rpc_pipe_fs.h
+++ b/include/linux/sunrpc/rpc_pipe_fs.h
@@ -46,6 +46,8 @@ RPC_I(struct inode *inode)
 
 extern struct vfsmount *init_rpc_pipefs;
 
+struct vfsmount *get_rpc_pipefs(const char *path);
+
 extern int rpc_queue_upcall(struct inode *, struct rpc_pipe_msg *);
 
 struct rpc_clnt;
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c
index b1e299b..4e09a90 100644
--- a/net/sunrpc/rpc_pipe.c
+++ b/net/sunrpc/rpc_pipe.c
@@ -16,6 +16,9 @@
 #include <linux/namei.h>
 #include <linux/fsnotify.h>
 #include <linux/kernel.h>
+#include <linux/nsproxy.h>
+#include <linux/mnt_namespace.h>
+#include <linux/fs_struct.h>
 
 #include <asm/ioctls.h>
 #include <linux/fs.h>
@@ -931,6 +934,54 @@ static const struct super_operations s_ops = {
 
 #define RPCAUTH_GSSMAGIC 0x67596969
 
+static int check_rpc_pipefs(struct vfsmount *mnt, void *arg)
+{
+	struct vfsmount **rpcmount = arg;
+	struct path path = {
+		.mnt = mnt,
+		.dentry = mnt->mnt_root,
+	};
+
+	if (!mnt->mnt_sb)
+		return 0;
+	if (mnt->mnt_sb->s_magic != RPCAUTH_GSSMAGIC)
+		return 0;
+
+	if (!path_is_under(&path, &current->fs->root))
+		return 0;
+
+	*rpcmount = mntget(mnt);
+	return 1;
+}
+
+struct vfsmount *get_rpc_pipefs(const char *p)
+{
+	int error;
+	struct vfsmount *rpcmount = ERR_PTR(-EINVAL);
+	struct path path;
+
+	if (!p) {
+		iterate_mounts(check_rpc_pipefs, &rpcmount,
+				current->nsproxy->mnt_ns->root);
+
+		if (IS_ERR(rpcmount) && (current->nsproxy->mnt_ns ==
+					init_task.nsproxy->mnt_ns))
+			return mntget(init_rpc_pipefs);
+
+		return rpcmount;
+	}
+
+	error = kern_path(p, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &path);
+	if (error)
+		return ERR_PTR(error);
+
+	check_rpc_pipefs(path.mnt, &rpcmount);
+	path_put(&path);
+
+	return rpcmount;
+}
+EXPORT_SYMBOL_GPL(get_rpc_pipefs);
+
 /*
  * We have a single directory with 1 node in it.
  */
-- 
1.7.3.4


^ permalink raw reply related

* Re: [PATCH] UDPCP Communication Protocol
From: Eric Dumazet @ 2010-12-31 12:00 UTC (permalink / raw)
  To: stefani; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293794758.2973.49.camel@edumazet-laptop>

Le vendredi 31 décembre 2010 à 12:25 +0100, Eric Dumazet a écrit :
> Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
> écrit :
> > +		if (!list_empty(&usk->destlist)) {
> > +			state->sk = (struct sock *)usk;
> > +			state->dest = list_first_entry(&usk->destlist,
> > +					struct udpcp_dest, list);
> > +			sock_hold(state->sk);
> > +
> > +			if (atomic_read(&state->sk->sk_refcnt) != 1) {
> > +				spin_unlock_irqrestore(&spinlock, flags);
> > +				return state;
> > +			}
> > +			atomic_dec(&state->sk->sk_refcnt);
> > +		}
> > +
> 
> I am trying to understand what you are doing here.
> 
> It seems racy to me.
> 
> Apparently, what you want is to take a reference only if actual
> sk_refcnt is not zero.
> 
> I suggest using atomic_inc_notzero(&state->sk->sk_refcnt) to avoid the
> race in atomic_dec().
> 
> 

Before you ask why its racy, this is because UDP sockets are RCU
protected, and RCU lookups depend on sk_refcnt being zero or not.

Doing an sk_refcnt increment/decrement opens a race window for the
concurrent lookups.

^ permalink raw reply

* Re: [PATCH] UDPCP Communication Protocol
From: Eric Dumazet @ 2010-12-31 11:54 UTC (permalink / raw)
  To: Stefani Seibold; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293794589.5285.16.camel@wall-e>

Le vendredi 31 décembre 2010 à 12:23 +0100, Stefani Seibold a écrit :
> Am Freitag, den 31.12.2010, 11:41 +0100 schrieb Eric Dumazet:
> > Le vendredi 31 décembre 2010 à 11:22 +0100, Stefani Seibold a écrit :
> > > Am Freitag, den 31.12.2010, 11:00 +0100 schrieb Eric Dumazet:
> > > > Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
> > > > écrit :
> > > > > From: Stefani Seibold <stefani@seibold.net>
> > > > > 
> > > > >  
> > > > >  /*
> > > > >   *	Handle MSG_ERRQUEUE
> > > > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > > > > index 2d3ded4..f9890a2 100644
> > > > > --- a/net/ipv4/udp.c
> > > > > +++ b/net/ipv4/udp.c
> > > > > @@ -1310,7 +1310,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> > > > >  	if (inet_sk(sk)->inet_daddr)
> > > > >  		sock_rps_save_rxhash(sk, skb->rxhash);
> > > > >  
> > > > > -	rc = ip_queue_rcv_skb(sk, skb);
> > > > > +	rc = sock_queue_rcv_skb(sk, skb);
> > > > 
> > > > Ouch... Care to explain why you changed this part ???
> > > > 
> > > > You just destroyed commit f84af32cbca70a intent, without any word in
> > > > your changelog. Making UDP slower, while others try to speed it must be
> > > > explained and advertised.
> > > >  
> > > > In general, we prefer a preliminary patch introducing all the changes in
> > > > current stack, then another one with the new protocol.
> > > > 
> > > 
> > > I reverted this for two reasons:
> > > 
> > > First ip_queue_rcv_skb drops the dst entry, which breaks the user land
> > > application which expect packet info after a
> > > 
> > > setsockopt(handle, IPPROTO_IP, IP_PKTINFO, &const_int_1, sizeof(int));
> > > 
> > > But for packets already in the queue this information will be lost. So
> > > it is a potential race condition.
> > > 
> > 
> > Exactly same race with packet filters. 
> > 
> > If your life depends on that, you must flush incoming queue _after_
> > issuing setsockopt(handle, IPPROTO_IP, IP_PKTINFO, &const_int_1,
> > sizeof(int)). So that all following packets have the information needed.
> > 
> > 
> 
> I though always that the linux kernel never breaks user land. This is a
> break!
> 

Only if user land is buggy it breaks. Where is your user land code so
that I can show you the bug ?

This dst refcount avoidance is absolutely crucial and we worked hard on
it.

> > 
> > > Second it breaks my UDPCP communication protocol stack module, which
> > > works very well till 2.6.35. I need this information in the data_ready()
> > > function to generate an ACK.
> > > 
> > > 
> > 
> > See now why you should not proceed like that ?
> > 
> > You know _perfectly_ there is a problem but prefer to keep it for you,
> > and hope this bit will be unnoticed ?
> > 
> 
> Stop to accuse me. There was a feature that was gone. An it took me six
> hours to figure out whats going wrong. I did not saw and see a real
> problem with this patch. It looked for me like an easy and clean
> solution. It was never my intention to trick somebody, especially u.
> 

Silently doing a revert is not an option. How must I tell this to you ?


> > This is not how things are dealed in linux, really.
> > 
> > You'll have to find a way so that things work well for everybody, not
> > only for you.
> > 
> > I guess you must fix UDPCP protocol stack, not 'fix linux'
> > 
> 
> I cannot fix it, because the information is still lost, and i need it. 
> 

You can fix it. Really. If not, you can pay me and I'll fix it for you.

> In my opinion it was a very bad idea to throw away important
> information. I checked it and Linux handle this since 2.6.0 in this way.
> 
> It would be better not to accuse than to work on a solution. 
> 

Where do you see an "accuse" ? Because you tried to silently "fix" the
thing without telling us how the damn thing was broken ? Come on !

> Question: How much performace gain does the early drop give. Are there
> benchmark results?

Thats pretty simple. dst refcount was the only contention point in UDP
stack. Yes, its not a joke.

Re introducing an atomic_inc() at each incoming packet, and atomic_dec()
each time user process dequeues the packet can have a huge impact.

One order of magnitude actually. Depending on number of cpus fighting on
this cache line, this ranges from 20% to 4000% slowdown.

Some people handle thousands of UDP sockets on one machine. Your UDPCP
apparently handle very few sockets (you have one central linked list),
so your use case probably dont care of performance.

^ permalink raw reply

* Re: [PATCH] UDPCP Communication Protocol
From: Eric Dumazet @ 2010-12-31 11:25 UTC (permalink / raw)
  To: stefani; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293787785-3834-1-git-send-email-stefani@seibold.net>

Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
écrit :
> +		if (!list_empty(&usk->destlist)) {
> +			state->sk = (struct sock *)usk;
> +			state->dest = list_first_entry(&usk->destlist,
> +					struct udpcp_dest, list);
> +			sock_hold(state->sk);
> +
> +			if (atomic_read(&state->sk->sk_refcnt) != 1) {
> +				spin_unlock_irqrestore(&spinlock, flags);
> +				return state;
> +			}
> +			atomic_dec(&state->sk->sk_refcnt);
> +		}
> +

I am trying to understand what you are doing here.

It seems racy to me.

Apparently, what you want is to take a reference only if actual
sk_refcnt is not zero.

I suggest using atomic_inc_notzero(&state->sk->sk_refcnt) to avoid the
race in atomic_dec().

^ permalink raw reply

* Re: [PATCH] UDPCP Communication Protocol
From: Stefani Seibold @ 2010-12-31 11:23 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293792066.2973.43.camel@edumazet-laptop>

Am Freitag, den 31.12.2010, 11:41 +0100 schrieb Eric Dumazet:
> Le vendredi 31 décembre 2010 à 11:22 +0100, Stefani Seibold a écrit :
> > Am Freitag, den 31.12.2010, 11:00 +0100 schrieb Eric Dumazet:
> > > Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
> > > écrit :
> > > > From: Stefani Seibold <stefani@seibold.net>
> > > > 
> > > >  
> > > >  /*
> > > >   *	Handle MSG_ERRQUEUE
> > > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > > > index 2d3ded4..f9890a2 100644
> > > > --- a/net/ipv4/udp.c
> > > > +++ b/net/ipv4/udp.c
> > > > @@ -1310,7 +1310,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> > > >  	if (inet_sk(sk)->inet_daddr)
> > > >  		sock_rps_save_rxhash(sk, skb->rxhash);
> > > >  
> > > > -	rc = ip_queue_rcv_skb(sk, skb);
> > > > +	rc = sock_queue_rcv_skb(sk, skb);
> > > 
> > > Ouch... Care to explain why you changed this part ???
> > > 
> > > You just destroyed commit f84af32cbca70a intent, without any word in
> > > your changelog. Making UDP slower, while others try to speed it must be
> > > explained and advertised.
> > >  
> > > In general, we prefer a preliminary patch introducing all the changes in
> > > current stack, then another one with the new protocol.
> > > 
> > 
> > I reverted this for two reasons:
> > 
> > First ip_queue_rcv_skb drops the dst entry, which breaks the user land
> > application which expect packet info after a
> > 
> > setsockopt(handle, IPPROTO_IP, IP_PKTINFO, &const_int_1, sizeof(int));
> > 
> > But for packets already in the queue this information will be lost. So
> > it is a potential race condition.
> > 
> 
> Exactly same race with packet filters. 
> 
> If your life depends on that, you must flush incoming queue _after_
> issuing setsockopt(handle, IPPROTO_IP, IP_PKTINFO, &const_int_1,
> sizeof(int)). So that all following packets have the information needed.
> 
> 

I though always that the linux kernel never breaks user land. This is a
break!

> 
> > Second it breaks my UDPCP communication protocol stack module, which
> > works very well till 2.6.35. I need this information in the data_ready()
> > function to generate an ACK.
> > 
> > 
> 
> See now why you should not proceed like that ?
> 
> You know _perfectly_ there is a problem but prefer to keep it for you,
> and hope this bit will be unnoticed ?
> 

Stop to accuse me. There was a feature that was gone. An it took me six
hours to figure out whats going wrong. I did not saw and see a real
problem with this patch. It looked for me like an easy and clean
solution. It was never my intention to trick somebody, especially u.

> This is not how things are dealed in linux, really.
> 
> You'll have to find a way so that things work well for everybody, not
> only for you.
> 
> I guess you must fix UDPCP protocol stack, not 'fix linux'
> 

I cannot fix it, because the information is still lost, and i need it. 

In my opinion it was a very bad idea to throw away important
information. I checked it and Linux handle this since 2.6.0 in this way.

It would be better not to accuse than to work on a solution. 

Question: How much performace gain does the early drop give. Are there
benchmark results?




^ permalink raw reply

* Re: [PATCH] UDPCP Communication Protocol
From: Eric Dumazet @ 2010-12-31 10:41 UTC (permalink / raw)
  To: Stefani Seibold; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293790979.4787.10.camel@wall-e>

Le vendredi 31 décembre 2010 à 11:22 +0100, Stefani Seibold a écrit :
> Am Freitag, den 31.12.2010, 11:00 +0100 schrieb Eric Dumazet:
> > Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
> > écrit :
> > > From: Stefani Seibold <stefani@seibold.net>
> > > 
> > >  
> > >  /*
> > >   *	Handle MSG_ERRQUEUE
> > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > > index 2d3ded4..f9890a2 100644
> > > --- a/net/ipv4/udp.c
> > > +++ b/net/ipv4/udp.c
> > > @@ -1310,7 +1310,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> > >  	if (inet_sk(sk)->inet_daddr)
> > >  		sock_rps_save_rxhash(sk, skb->rxhash);
> > >  
> > > -	rc = ip_queue_rcv_skb(sk, skb);
> > > +	rc = sock_queue_rcv_skb(sk, skb);
> > 
> > Ouch... Care to explain why you changed this part ???
> > 
> > You just destroyed commit f84af32cbca70a intent, without any word in
> > your changelog. Making UDP slower, while others try to speed it must be
> > explained and advertised.
> >  
> > In general, we prefer a preliminary patch introducing all the changes in
> > current stack, then another one with the new protocol.
> > 
> 
> I reverted this for two reasons:
> 
> First ip_queue_rcv_skb drops the dst entry, which breaks the user land
> application which expect packet info after a
> 
> setsockopt(handle, IPPROTO_IP, IP_PKTINFO, &const_int_1, sizeof(int));
> 
> But for packets already in the queue this information will be lost. So
> it is a potential race condition.
> 

Exactly same race with packet filters. 

If your life depends on that, you must flush incoming queue _after_
issuing setsockopt(handle, IPPROTO_IP, IP_PKTINFO, &const_int_1,
sizeof(int)). So that all following packets have the information needed.



> Second it breaks my UDPCP communication protocol stack module, which
> works very well till 2.6.35. I need this information in the data_ready()
> function to generate an ACK.
> 
> 

See now why you should not proceed like that ?

You know _perfectly_ there is a problem but prefer to keep it for you,
and hope this bit will be unnoticed ?

This is not how things are dealed in linux, really.

You'll have to find a way so that things work well for everybody, not
only for you.

I guess you must fix UDPCP protocol stack, not 'fix linux'

^ permalink raw reply

* Re: [PATCH] UDPCP Communication Protocol
From: Eric Dumazet @ 2010-12-31 10:35 UTC (permalink / raw)
  To: stefani; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293787785-3834-1-git-send-email-stefani@seibold.net>

Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
écrit :
> 		/*
> +			 * set a flag for UDPCP message
> +			 */
> +			skb->cb[sizeof(struct udp_skb_cb)] = 1;


Dont do that. This hides an important thing : you use one extra byte in
skb->cb[] without being explicit.

As we have one byte hole in struct udp_skb_cbn, you could use it
instead.

diff --git a/include/net/udp.h b/include/net/udp.h
index bb967dd..ceafbbf 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -47,6 +47,7 @@ struct udp_skb_cb {
 	} header;
 	__u16		cscov;
 	__u8		partial_cov;
+	__u8		udpcp_flag;
 };
 #define UDP_SKB_CB(__skb)	((struct udp_skb_cb *)((__skb)->cb))
 

^ permalink raw reply related

* Re: [PATCH 01/15]arch:m68k:ifpsp060:src:fpsp.S Typo change diable to disable.
From: Geert Uytterhoeven @ 2010-12-31 10:33 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: trivial, linux-m68k, linux-kernel, netdev, ivtv-devel,
	linux-media, linux-wireless, linux-scsi, spi-devel-general, devel,
	linux-usb
In-Reply-To: <1293750484-1161-1-git-send-email-justinmattock@gmail.com>

On Fri, Dec 31, 2010 at 00:07, Justin P. Mattock
<justinmattock@gmail.com> wrote:
> The below patch fixes a typo "diable" to "disable". Please let me know if this
> is correct or not.
>
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

> ---
>  arch/m68k/ifpsp060/src/fpsp.S |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/m68k/ifpsp060/src/fpsp.S b/arch/m68k/ifpsp060/src/fpsp.S
> index 73613b5..26e85e2 100644
> --- a/arch/m68k/ifpsp060/src/fpsp.S
> +++ b/arch/m68k/ifpsp060/src/fpsp.S
> @@ -3881,7 +3881,7 @@ _fpsp_fline:
>  # FP Unimplemented Instruction stack frame and jump to that entry
>  # point.
>  #
> -# but, if the FPU is disabled, then we need to jump to the FPU diabled
> +# but, if the FPU is disabled, then we need to jump to the FPU disabled
>  # entry point.
>        movc            %pcr,%d0
>        btst            &0x1,%d0

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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] UDPCP Communication Protocol
From: Stefani Seibold @ 2010-12-31 10:29 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293790501.2973.33.camel@edumazet-laptop>

Am Freitag, den 31.12.2010, 11:15 +0100 schrieb Eric Dumazet:
> Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
> écrit :
> > +				spin_lock_irqsave(&spinlock, flags);
> > +				udpcp_stat.txMsgs++;
> > +				spin_unlock_irqrestore(&spinlock, flags);
> 
> This is really ugly for different reasons :
> 
> 1) Naming a lock, even static "spinlock" is ugly.

Agree...

> 2) Using a lock for stats is not necessary, and
>    disabling hard irqs is not necessary either (spinlock_bh() would be
> more than enough)
>   
>    At a very minimum, you should use atomic_t so that no lock is needed
> 
> 3) Network stack widely use MIB per_cpu counters.
>  As you use UDP, you could take a look at UDP_INC_STATS_BH()/
> UDP_INC_STATS_USER() implementation for an example.
> 

I will have look at this and revamp it.

^ permalink raw reply

* Re: [PATCH 14/15]include:media:davinci:vpss.h Typo change diable to disable.
From: Mauro Carvalho Chehab @ 2010-12-31 10:27 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: trivial, linux-m68k, linux-kernel, netdev, ivtv-devel,
	linux-media, linux-wireless, linux-scsi, spi-devel-general, devel,
	linux-usb
In-Reply-To: <1293750484-1161-14-git-send-email-justinmattock@gmail.com>

Em 30-12-2010 21:08, Justin P. Mattock escreveu:
> The below patch fixes a typo "diable" to "disable". Please let me know if this 
> is correct or not.
> 
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>

PS.: Next time, please c/c linux-media ONLY on patches related to media
drivers (/drivers/video and the corresponding include files). Having to
dig into a series of 15 patches to just actually look on 3 patches 
is not nice.

> 
> ---
>  include/media/davinci/vpss.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/media/davinci/vpss.h b/include/media/davinci/vpss.h
> index c59cc02..b586495 100644
> --- a/include/media/davinci/vpss.h
> +++ b/include/media/davinci/vpss.h
> @@ -44,7 +44,7 @@ struct vpss_pg_frame_size {
>  	short pplen;
>  };
>  
> -/* Used for enable/diable VPSS Clock */
> +/* Used for enable/disable VPSS Clock */
>  enum vpss_clock_sel {
>  	/* DM355/DM365 */
>  	VPSS_CCDC_CLOCK,

^ permalink raw reply

* Re: [PATCH/RFC] Re: Compilation of pktgen fails for ARCH=um
From: Christoph Paasch @ 2010-12-31 10:26 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: netdev, linux-arch
In-Reply-To: <20101230093925.3d6b567e.randy.dunlap@oracle.com>

Hi,

On Thursday, December 30, 2010 wrote Randy Dunlap:
> or ndelay() in arch/um/include/asm/delay.y can be removed completely
> and then the default implementation of ndelay() will be used from
> include/linux/delay.h.
> This builds cleanly, but I don't know how well it would work.
This will do a rounded up micro-second sleep. I think that it's ok.

Christoph

> ---
> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> Allow uml to use the default implementation of ndelay() from
> <linux/delay.h>.  Fixes build error:
> 
> net/built-in.o: In function `spin':
> pktgen.c:(.text+0x27391): undefined reference to `__unimplemented_ndelay'
> 
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> ---
>  arch/um/include/asm/delay.h |    5 -----
>  1 file changed, 5 deletions(-)
> 
> --- linux-next-20101214.orig/arch/um/include/asm/delay.h
> +++ linux-next-20101214/arch/um/include/asm/delay.h
> @@ -12,9 +12,4 @@ extern void __delay(unsigned long loops)
>  #define udelay(n) ((__builtin_constant_p(n) && (n) > 20000) ? \
>  	__bad_udelay() : __udelay(n))
> 
> -/* It appears that ndelay is not used at all for UML, and has never been
> - * implemented. */
> -extern void __unimplemented_ndelay(void);
> -#define ndelay(n) __unimplemented_ndelay()
> -
>  #endif

--
Christoph Paasch
PhD Student

IP Networking Lab --- http://inl.info.ucl.ac.be
MultiPath TCP in the Linux Kernel --- http://inl.info.ucl.ac.be/mptcp
Université Catholique de Louvain

www.rollerbulls.be
--

^ permalink raw reply

* Re: [PATCH 12/15]drivers:media:video:tvp7002.c Typo change diable to disable.
From: Mauro Carvalho Chehab @ 2010-12-31 10:24 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: devel, linux-m68k, trivial, linux-scsi, netdev, linux-usb,
	linux-wireless, linux-kernel, ivtv-devel, spi-devel-general,
	linux-media
In-Reply-To: <1293750484-1161-12-git-send-email-justinmattock@gmail.com>

Em 30-12-2010 21:08, Justin P. Mattock escreveu:
> The below patch fixes a typo "diable" to "disable". Please let me know if this 
> is correct or not.
> 
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> 
> ---
>  drivers/media/video/tvp7002.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/tvp7002.c b/drivers/media/video/tvp7002.c
> index e63b40f..c799e4e 100644
> --- a/drivers/media/video/tvp7002.c
> +++ b/drivers/media/video/tvp7002.c
> @@ -789,7 +789,7 @@ static int tvp7002_query_dv_preset(struct v4l2_subdev *sd,
>   * Get the value of a TVP7002 decoder device register.
>   * Returns zero when successful, -EINVAL if register read fails or
>   * access to I2C client fails, -EPERM if the call is not allowed
> - * by diabled CAP_SYS_ADMIN.
> + * by disabled CAP_SYS_ADMIN.
>   */
>  static int tvp7002_g_register(struct v4l2_subdev *sd,
>  						struct v4l2_dbg_register *reg)

^ permalink raw reply

* Re: [PATCH 11/15]drivers:media:video:cx18:cx23418.h Typo change diable to disable.
From: Mauro Carvalho Chehab @ 2010-12-31 10:23 UTC (permalink / raw)
  To: Justin P. Mattock
  Cc: trivial, linux-m68k, linux-kernel, netdev, ivtv-devel,
	linux-media, linux-wireless, linux-scsi, spi-devel-general, devel,
	linux-usb
In-Reply-To: <1293750484-1161-11-git-send-email-justinmattock@gmail.com>

Em 30-12-2010 21:08, Justin P. Mattock escreveu:
> The below patch fixes a typo "diable" to "disable". Please let me know if this 
> is correct or not.
> 
> Signed-off-by: Justin P. Mattock <justinmattock@gmail.com>
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
> 
> ---
>  drivers/media/video/cx18/cx23418.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/cx18/cx23418.h b/drivers/media/video/cx18/cx23418.h
> index 2c00980..7e40035 100644
> --- a/drivers/media/video/cx18/cx23418.h
> +++ b/drivers/media/video/cx18/cx23418.h
> @@ -177,7 +177,7 @@
>     IN[0] - Task handle.
>     IN[1] - luma type: 0 = disable, 1 = 1D horizontal only, 2 = 1D vertical only,
>  		      3 = 2D H/V separable, 4 = 2D symmetric non-separable
> -   IN[2] - chroma type: 0 - diable, 1 = 1D horizontal
> +   IN[2] - chroma type: 0 - disable, 1 = 1D horizontal
>     ReturnCode - One of the ERR_CAPTURE_... */
>  #define CX18_CPU_SET_SPATIAL_FILTER_TYPE     	(CPU_CMD_MASK_CAPTURE | 0x000C)
>  

^ permalink raw reply

* Re: [PATCH] UDPCP Communication Protocol
From: Stefani Seibold @ 2010-12-31 10:22 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293789629.2973.26.camel@edumazet-laptop>

Am Freitag, den 31.12.2010, 11:00 +0100 schrieb Eric Dumazet:
> Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
> écrit :
> > From: Stefani Seibold <stefani@seibold.net>
> > 
> >  
> >  /*
> >   *	Handle MSG_ERRQUEUE
> > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > index 2d3ded4..f9890a2 100644
> > --- a/net/ipv4/udp.c
> > +++ b/net/ipv4/udp.c
> > @@ -1310,7 +1310,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
> >  	if (inet_sk(sk)->inet_daddr)
> >  		sock_rps_save_rxhash(sk, skb->rxhash);
> >  
> > -	rc = ip_queue_rcv_skb(sk, skb);
> > +	rc = sock_queue_rcv_skb(sk, skb);
> 
> Ouch... Care to explain why you changed this part ???
> 
> You just destroyed commit f84af32cbca70a intent, without any word in
> your changelog. Making UDP slower, while others try to speed it must be
> explained and advertised.
>  
> In general, we prefer a preliminary patch introducing all the changes in
> current stack, then another one with the new protocol.
> 

I reverted this for two reasons:

First ip_queue_rcv_skb drops the dst entry, which breaks the user land
application which expect packet info after a

setsockopt(handle, IPPROTO_IP, IP_PKTINFO, &const_int_1, sizeof(int));

But for packets already in the queue this information will be lost. So
it is a potential race condition.

Second it breaks my UDPCP communication protocol stack module, which
works very well till 2.6.35. I need this information in the data_ready()
function to generate an ACK.

^ permalink raw reply

* RE: [PATCH net-2.6] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: Johannes Berg @ 2010-12-31 10:18 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: Stephen Hemminger, Stephen Hemminger,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <6F5C1D715B2DA5498A628E6B9C124F04019BF36ABD-KS4eWWg9cz+vNW/NfzhIbrfspsVTdybXVpNB7YpNyf8@public.gmane.org>

On Fri, 2010-12-31 at 01:29 +0200, Winkler, Tomas wrote:
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA@public.gmane.org]
> > Sent: Friday, December 31, 2010 1:06 AM
> > To: Winkler, Tomas; Stephen Hemminger; Johannes Berg
> > Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org ; linux-
> > wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Subject: RE: [PATCH net-2.6] bridge: fix br_multicast_ipv6_rcv for paged
> > skbs
> > 
> > Although copy is slower for large packets, this is a non performance path.
> > The code in question is for bridged multicast Ipv6 ICMP packets. This case
> > is so uncritical it could be done in BASIC and no one could possibly care!
> > 
> 
> 
> Fair enough, although you got few of those when you connect to win7 client. 
> Anyhow my fix would work if the second pull would be 
>   if (!pskb_may_pull(skb2, sizeof(struct mld_msg)))  instead of  (!pskb_may_pull(skb2, sizeof(*icmp6h)))

I don't think that works either since that may be longer than the entire
skb's length since the payload still is variable at this point.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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] UDPCP Communication Protocol
From: Eric Dumazet @ 2010-12-31 10:15 UTC (permalink / raw)
  To: stefani; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293787785-3834-1-git-send-email-stefani@seibold.net>

Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
écrit :
> +				spin_lock_irqsave(&spinlock, flags);
> +				udpcp_stat.txMsgs++;
> +				spin_unlock_irqrestore(&spinlock, flags);

This is really ugly for different reasons :

1) Naming a lock, even static "spinlock" is ugly.
2) Using a lock for stats is not necessary, and
   disabling hard irqs is not necessary either (spinlock_bh() would be
more than enough)
  
   At a very minimum, you should use atomic_t so that no lock is needed

3) Network stack widely use MIB per_cpu counters.
 As you use UDP, you could take a look at UDP_INC_STATS_BH()/
UDP_INC_STATS_USER() implementation for an example.

^ permalink raw reply

* Re: [PATCH] UDPCP Communication Protocol
From: Eric Dumazet @ 2010-12-31 10:00 UTC (permalink / raw)
  To: stefani; +Cc: linux-kernel, akpm, davem, netdev
In-Reply-To: <1293787785-3834-1-git-send-email-stefani@seibold.net>

Le vendredi 31 décembre 2010 à 10:29 +0100, stefani@seibold.net a
écrit :
> From: Stefani Seibold <stefani@seibold.net>
> 
> UDPCP is a communication protocol specified by the Open Base Station
> Architecture Initiative Special Interest Group (OBSAI SIG). The
> protocol is based on UDP and is designed to meet the needs of "Mobile
> Communcation Base Station" internal communications. It is widely used by
> the major networks infrastructure supplier.
> 
> The UDPCP communication service supports the following features:
> 
> -Connectionless communication for serial mode data transfer
> -Acknowledged and unacknowledged transfer modes
> -Retransmissions Algorithm
> -Checksum Algorithm using Adler32
> -Fragmentation of long messages (disassembly/reassembly) to match to the MTU
>  during transport:
> -Broadcasting and multicasting messages to multiple peers in unacknowledged
>  transfer mode
> 
> UDPCP supports application level messages up to 64 KBytes (limited by 16-bit
> packet data length field). Messages that are longer than the MTU will be
> fragmented to the MTU.
> 
> UDPCP provides a reliable transport service that will perform message
> retransmissions in case transport failures occur.
> 
> The code is also a nice example how to implement a UDP based protocol as
> a kernel socket modules.
> 
> Due the nature of UDPCP which has no sliding windows support, the latency has a
> huge impact. The perfomance increase by implementing as a kernel module is
> about the factor 10, because there are no context switches and data packets or
> ACKs will be handled in the interrupt service.
> 
> There are no side effects to the network subsystems so i ask for merge it
> into linux-next. Hope you like it.
> 
> Wish a happy new year. Keep on hacking.
> 
> - Stefani
> 
> Signed-off-by: Stefani Seibold <stefani@seibold.net>
> ---
>  include/linux/socket.h |    5 +-
>  include/net/udpcp.h    |   47 +
>  net/Kconfig            |    1 +
>  net/Makefile           |    1 +
>  net/ipv4/ip_output.c   |    2 +
>  net/ipv4/ip_sockglue.c |    2 +
>  net/ipv4/udp.c         |    2 +-
>  net/udpcp/Kconfig      |   34 +
>  net/udpcp/Makefile     |    5 +
>  net/udpcp/udpcp.c      | 2883 ++++++++++++++++++++++++++++++++++++++++++++++++
>  10 files changed, 2980 insertions(+), 2 deletions(-)
>  create mode 100644 include/net/udpcp.h
>  create mode 100644 net/udpcp/Kconfig
>  create mode 100644 net/udpcp/Makefile
>  create mode 100644 net/udpcp/udpcp.c
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 86b652f..624c5ed 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -193,7 +193,8 @@ struct ucred {
>  #define AF_PHONET	35	/* Phonet sockets		*/
>  #define AF_IEEE802154	36	/* IEEE802154 sockets		*/
>  #define AF_CAIF		37	/* CAIF sockets			*/
> -#define AF_MAX		38	/* For now.. */
> +#define	AF_UDPCP	38	/* UDPCP sockets		*/
> +#define AF_MAX		39	/* For now.. */
>  
>  /* Protocol families, same as address families. */
>  #define PF_UNSPEC	AF_UNSPEC
> @@ -234,6 +235,7 @@ struct ucred {
>  #define PF_PHONET	AF_PHONET
>  #define PF_IEEE802154	AF_IEEE802154
>  #define PF_CAIF		AF_CAIF
> +#define	PF_UDPCP	AF_UDPCP
>  #define PF_MAX		AF_MAX
>  
>  /* Maximum queue length specifiable by listen.  */
> @@ -307,6 +309,7 @@ struct ucred {
>  #define SOL_RDS		276
>  #define SOL_IUCV	277
>  #define SOL_CAIF	278
> +#define SOL_UDPCP	279
>  
>  /* IPX options */
>  #define IPX_TYPE	1
> diff --git a/include/net/udpcp.h b/include/net/udpcp.h
> new file mode 100644
> index 0000000..ba199b9
> --- /dev/null
> +++ b/include/net/udpcp.h
> @@ -0,0 +1,47 @@
> +/* Definitions for UDPCP sockets. */
> +
> +#ifndef __LINUX_IF_UDPCP
> +#define __LINUX_IF_UDPCP
> +
> +#include "linux/ioctl.h"
> +
> +#define UDPCP_MAX_MSGSIZE	65487
> +
> +#define	UDPCP_MAX_WAIT_SEC	60
> +
> +#define UDPCP_OPT_TRANSFER_MODE		0
> +#define UDPCP_OPT_CHECKSUM_MODE		1
> +#define UDPCP_OPT_TX_TIMEOUT		2
> +#define UDPCP_OPT_RX_TIMEOUT		3
> +#define UDPCP_OPT_MAXTRY		4
> +#define	UDPCP_OPT_OUTSTANDING_ACKS	5
> +
> +#define	UDPCP_NOACK		0
> +#define	UDPCP_ACK		1
> +#define	UDPCP_SINGLE_ACK	2
> +#define	UDPCP_NOCHECKSUM	3
> +#define	UDPCP_CHECKSUM		4
> +
> +#define UDPCP_IOC_MAGIC  251
> +
> +#define UDPCP_IOCTL_GET_STATISTICS \
> +	_IOR(UDPCP_IOC_MAGIC, 0x01, struct udpcp_statistics *)
> +#define UDPCP_IOCTL_RESET_STATISTICS \
> +	_IO(UDPCP_IOC_MAGIC, 0x02)
> +#define UDPCP_IOCTL_SYNC \
> +	_IOR(UDPCP_IOC_MAGIC, 0x03, unsigned long)
> +
> +struct udpcp_statistics {
> +	unsigned int txMsgs;		/* Num of transmitted messages */
> +	unsigned int rxMsgs;		/* Num of received messages */
> +	unsigned int txNodes;		/* Num of receiver nodes */
> +	unsigned int rxNodes;		/* Num of transmitter nodes */
> +	unsigned int txTimeout;		/* Num of unsuccessful transmissions */
> +	unsigned int rxTimeout;		/* Num of partial message receptions */
> +	unsigned int txRetries;		/* Num of resends */
> +	unsigned int rxDiscardedFrags;	/* Num of discarded fragments */
> +	unsigned int crcErrors;		/* Num of crc errors detected */
> +};
> +
> +#endif
> +
> diff --git a/net/Kconfig b/net/Kconfig
> index 55fd82e..4a206fc 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -294,6 +294,7 @@ source "net/rfkill/Kconfig"
>  source "net/9p/Kconfig"
>  source "net/caif/Kconfig"
>  source "net/ceph/Kconfig"
> +source "net/udpcp/Kconfig"
>  
> 
>  endif   # if NET
> diff --git a/net/Makefile b/net/Makefile
> index 6b7bfd7..a17ae27 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -69,3 +69,4 @@ endif
>  obj-$(CONFIG_WIMAX)		+= wimax/
>  obj-$(CONFIG_DNS_RESOLVER)	+= dns_resolver/
>  obj-$(CONFIG_CEPH_LIB)		+= ceph/
> +obj-$(CONFIG_UDPCP)		+= udpcp/
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 439d2a3..55b2d0c 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1085,6 +1085,7 @@ error:
>  	IP_INC_STATS(sock_net(sk), IPSTATS_MIB_OUTDISCARDS);
>  	return err;
>  }
> +EXPORT_SYMBOL(ip_append_data);
>  
>  ssize_t	ip_append_page(struct sock *sk, struct page *page,
>  		       int offset, size_t size, int flags)
> @@ -1341,6 +1342,7 @@ error:
>  	IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
>  	goto out;
>  }
> +EXPORT_SYMBOL(ip_push_pending_frames);
>  
>  /*
>   *	Throw away all pending data on the socket.
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index 3948c86..310369c 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -226,6 +226,7 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc)
>  	}
>  	return 0;
>  }
> +EXPORT_SYMBOL(ip_cmsg_send);
>  
> 
>  /* Special input handler for packets caught by router alert option.
> @@ -369,6 +370,7 @@ void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 port, u32 inf
>  	if (sock_queue_err_skb(sk, skb))
>  		kfree_skb(skb);
>  }
> +EXPORT_SYMBOL(ip_local_error);
>  
>  /*
>   *	Handle MSG_ERRQUEUE
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 2d3ded4..f9890a2 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1310,7 +1310,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
>  	if (inet_sk(sk)->inet_daddr)
>  		sock_rps_save_rxhash(sk, skb->rxhash);
>  
> -	rc = ip_queue_rcv_skb(sk, skb);
> +	rc = sock_queue_rcv_skb(sk, skb);

Ouch... Care to explain why you changed this part ???

You just destroyed commit f84af32cbca70a intent, without any word in
your changelog. Making UDP slower, while others try to speed it must be
explained and advertised.
 
In general, we prefer a preliminary patch introducing all the changes in
current stack, then another one with the new protocol.

^ 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