* [PATCH kernel 2.6.33-git11] lib8390: use spin_lock_irqsave for locking
From: Ken Kawasaki @ 2010-03-06 22:02 UTC (permalink / raw)
To: netdev
In-Reply-To: <20091213094411.18be2ff6.ken_kawasaki@spring.nifty.jp>
lib8390:
use "spin_lock_irqsave", "local_irq_save" instead of "disable_irq".
Signed-off-by: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>
---
--- linux-2.6.33-git11/drivers/net/lib8390.c.orig 2010-03-06 20:09:28.000000000 +0900
+++ linux-2.6.33-git11/drivers/net/lib8390.c 2010-03-07 06:42:47.000000000 +0900
@@ -279,15 +279,13 @@ static void __ei_tx_timeout(struct net_d
/* Ugly but a reset can be slow, yet must be protected */
- disable_irq_nosync_lockdep(dev->irq);
- spin_lock(&ei_local->page_lock);
+ spin_lock_irqsave(&ei_local->page_lock, flags);
/* Try to restart the card. Perhaps the user has fixed something. */
ei_reset_8390(dev);
__NS8390_init(dev, 1);
- spin_unlock(&ei_local->page_lock);
- enable_irq_lockdep(dev->irq);
+ spin_unlock_irqrestore(&ei_local->page_lock, flags);
netif_wake_queue(dev);
}
@@ -323,17 +321,11 @@ static netdev_tx_t __ei_start_xmit(struc
spin_lock_irqsave(&ei_local->page_lock, flags);
ei_outb_p(0x00, e8390_base + EN0_IMR);
- spin_unlock_irqrestore(&ei_local->page_lock, flags);
-
/*
* Slow phase with lock held.
*/
- disable_irq_nosync_lockdep_irqsave(dev->irq, &flags);
-
- spin_lock(&ei_local->page_lock);
-
ei_local->irqlock = 1;
/*
@@ -368,8 +360,7 @@ static netdev_tx_t __ei_start_xmit(struc
ei_local->irqlock = 0;
netif_stop_queue(dev);
ei_outb_p(ENISR_ALL, e8390_base + EN0_IMR);
- spin_unlock(&ei_local->page_lock);
- enable_irq_lockdep_irqrestore(dev->irq, &flags);
+ spin_unlock_irqrestore(&ei_local->page_lock, flags);
dev->stats.tx_errors++;
return NETDEV_TX_BUSY;
}
@@ -409,8 +400,7 @@ static netdev_tx_t __ei_start_xmit(struc
ei_local->irqlock = 0;
ei_outb_p(ENISR_ALL, e8390_base + EN0_IMR);
- spin_unlock(&ei_local->page_lock);
- enable_irq_lockdep_irqrestore(dev->irq, &flags);
+ spin_unlock_irqrestore(&ei_local->page_lock, flags);
dev_kfree_skb (skb);
dev->stats.tx_bytes += send_length;
@@ -526,9 +516,11 @@ static irqreturn_t __ei_interrupt(int ir
#ifdef CONFIG_NET_POLL_CONTROLLER
static void __ei_poll(struct net_device *dev)
{
- disable_irq(dev->irq);
+ unsigned long flags;
+
+ local_irq_save(flags);
__ei_interrupt(dev->irq, dev);
- enable_irq(dev->irq);
+ local_irq_restore(flags);
}
#endif
^ permalink raw reply
* Re: [PATCH 2/6] C/R: Basic support for network namespaces and devices (v5)
From: Oren Laadan @ 2010-03-06 22:21 UTC (permalink / raw)
To: Dan Smith
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
containers-qjLDD68F18O7TbgM5vRIOg, den-GEFAQzZX7r8dnm+yROfE0A,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, benjamin.thery-6ktuUTfB/bM
In-Reply-To: <87bpf1idic.fsf-FLMGYpZoEPULwtHQx/6qkW3U47Q5hpJU@public.gmane.org>
Dan Smith wrote:
> OL> What about leak detection ?
> OL> Aren't we missing {netns,netdev}_users()?
>
> This is something I need to give more thought to, but it's not as easy
> as it sounds. Network devices aren't released at the last put() like
> a lot of other things, and my initial attempts to reconcile the
> refcount after a checkpoint operation have not been successful.
>
> However, I'm not sure that it's as important here, because AFAIK, a
> network device can only exist in one network namespace at a time. If
> we're checkpointing a netdev, it's because we are checkpointing the
> namespace that it lives in. Making sure the netns isn't leaked out of
> the process tree would be much easier and just as effective, no?
We should guarantee that neither netns nor netdev leaks outside
the container; currently none is. If a netdev can only belong to
a single netns, then it suffices to only care about netns.
>
>>> +config CHECKPOINT_NETNS
>>> + bool
>>> + default y if NET && NET_NS && CHECKPOINT
>>> +
>
> OL> Did you mean this to be visible (settable) by the user ?
>
> No, it was specifically supposed to enable itself when those other
> items are enabled, but not be a user adjustable toggle. I had a
> discussion with Serge about it and we came to this as a solution,
> although I don't remember what the problem we started with was. I'll
> dig through my IRC logs to see if I can figure it out.
Duh.. my bad, I misinterpreted the code. That's fine.
BTW, there is a similar SYSVIPC_CHECKPOINT - we should decide
if we do X_CHECKPOINT or CHECKPOINT_X for a subsystem X, and
stick to that convention. I prefer the latter - what you did...
>
>>> + retry:
>>> + if (++pages > 4) {
>>> + addrs = -E2BIG;
>>> + goto out;
>>> + }
>
> OL> Why 4 ?
>
> It's just a sanity limit.
Hmm... let me be more explicit: why not keep trying until it
realloc fails ? or switch to vmalloc() at some point ?
>
> OL> Do we really need this special case ? I'd be happy with a ckpt_err()
> OL> for any error - and the actual error number would be useful to tell
> OL> which case it was.
>
> Unless I'm missing something, you asked for this specifically:
>
> https://lists.linux-foundation.org/pipermail/containers/2010-February/022844.html
Lol .. that was me :o But looking at the code it feels wrong,
because the errno already reveals the type of the problem.
I'm thinking - wouldn't it make sense to do error reporting
in checkpoint_netdev() if the call to ->ndo_checkpoint() fails ?
>
> OL> Isn't this check redundant ? I expect it to fail promptly in
> OL> checkpoint_netdev() above.
>
> No, as I've said a couple of times previously, this isn't the only way
> we can arrive at a netdev for checkpointing. This case is the one
> where we're marching through the netns and find a netdev that is not
> supported. The other is where we arrive at a device as a peer of
> another device, so the other check may come into play at times where
> this one doesn't and vice versa.
I'm confused: in checkpoint_ns() inside the for_each_netdev()
loop you first test for dev->netdev_ops->ndo_checkpoint and
then call checkpoint_obj(... CKPT_OBJ_NETDEV) - which in turn
will call checkpoint_netdev(), which will again test for
dev->netdev_ops->ndo_checkpoint ... am I reading it wrongly ?
>
> OL> This may be a bit simpler if you move the first deferqueue_add()
> OL> forward to just before the other one. Or better: change dq_netdev
> OL> to have two pointers, dev and peer (if any is null, the cleanup
> OL> function will skip).
>
> The reason it is this messy is because of the way network devices are
> deallocated. Since they don't destroy themselves on the final put(),
> we have to explicitly call unregister_netdev() on them when we know
> they're no longer used (else we block). Once we've added them to the
> deferqueue, we can no longer destroy them here because a reference is
> held and the deferqueue will run afterwards.
>
> The ordering of this is a result of me injecting failures at each step
> and working it out until I got it to not block on unregistering either
> of the devices in all of the error paths. That's not to say it's the
> best way, but there is a reason it's ordered the way it is.
>
How about this - to me it feels simpler:
dev = rtnl_newlink(veth_new_link_msg, &veth, this_name);
if (IS_ERR(dev))
return dev;
peer = dev_get_by_name(current->nsproxy->net_ns, peer_name);
if (!peer) {
ret = -EINVAL;
goto err_dev;
}
ret = ckpt_obj_insert(ctx, peer, h->veth.peer_ref,
CKPT_OBJ_NETDEV);
if (ret < 0)
goto err_peer;
dev_put(peer);
dq.dev = dev;
dq.peer = peer;
ret = deferqueue_add(ctx->deferqueue, &dq, sizeof(dq),
netdev_noop, netdev_cleanup);
if (ret)
goto err_peer;
(yes, you need to adjust struct dq_netdev and netdev_cleanup).
BTW, the variable "didreg" should disappear from restore_veth().
Oren.
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Joe Perches @ 2010-03-06 22:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061356420.31447@localhost.localdomain>
On Sat, 2010-03-06 at 14:03 -0800, Linus Torvalds wrote:
> On Sat, 6 Mar 2010, Joe Perches wrote:
> > Is that an ack, a nack or a get the hell out?
> It's mostly an Ack. I think the concept is great. I'd love to have some
> way to limit recursion,
Maybe limit the %pV recursion depth to 1 with something like:
(in vsprintf.c: pointer() )
case 'V': {
char *rtn;
static int rcount++;
if (rcount > 1) { /* Don't recurse more than once */
rcount--;
break; /* Use print the pointer */
}
rtn = buf + vsnprintf(buf, end - buf,
((struct va_format *)ptr)->fmt,
*(((struct va_format *)ptr)->va));
rcount--;
return rtn;
}
> and I'd also love to see some actual numbers of
> how deep the vsnprintf stack frame is, but I don't see how to do the
> first, and I'm hoping the second isn't too horrible.
I believe it's the arguments, a long long, a couple of pointers,
and a struct printf_spec. Not too bad.
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-06 22:52 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <1267914654.849.81.camel@Joe-Laptop.home>
On Sat, 6 Mar 2010, Joe Perches wrote:
>
> Maybe limit the %pV recursion depth to 1 with something like:
> (in vsprintf.c: pointer() )
Nope. Think about concurrent users.
The thing is, you have to hide it in local storage. We could make it
thread-local (not cpu-local), but even that interacts badly with
interrupts.
The only really workable approach would be to have a stack slot that is
created by the externally visible routines (and initialized to zero), and
those then passe the address of that as an argument to the lower levels,
and then the recursion happens entirely within those lower level functions
that update the value.
So it's doable, it's just not pretty.
> > and I'd also love to see some actual numbers of > > how deep the vsnprintf stack frame is, but I don't see how to do the
> > first, and I'm hoping the second isn't too horrible.
>
> I believe it's the arguments, a long long, a couple of pointers,
> and a struct printf_spec. Not too bad.
I'm not convinced. We pass that 'printf_spec' around a lot, including
nesting. Not as a pointer, either.
(Bjorn Helgaas has a patch that gets rid of _some_ of the stack usage, but
not nearly all).
Linus
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-06 22:57 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061445490.31447@localhost.localdomain>
On Sat, 6 Mar 2010, Linus Torvalds wrote:
>
> I'm not convinced. We pass that 'printf_spec' around a lot, including
> nesting. Not as a pointer, either.
Btw, I wonder if those fields could become bitfields, or 'unsigned char'
etc. That printf_spec really is unnecessarily big. It should _easily_ fit
in a single register on a 64-bit platform, possibly even a 32-bit one (we
could limit field width and precision to 5 bits, for example)
Linus
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Joe Perches @ 2010-03-06 23:35 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061455590.31447@localhost.localdomain>
On Sat, 2010-03-06 at 14:57 -0800, Linus Torvalds wrote:
>
> On Sat, 6 Mar 2010, Linus Torvalds wrote:
> >
> > I'm not convinced. We pass that 'printf_spec' around a lot, including
> > nesting. Not as a pointer, either.
>
> Btw, I wonder if those fields could become bitfields, or 'unsigned char'
> etc. That printf_spec really is unnecessarily big. It should _easily_ fit
> in a single register on a 64-bit platform, possibly even a 32-bit one (we
> could limit field width and precision to 5 bits, for example)
I believe 32 bits isn't possible.
type:6
flags:8
width:8
base:6
precision:6
qualifier:8
So maybe something like this:
lib/vsprintf.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
---
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index af4aaa6..fdee7f7 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -408,12 +408,12 @@ enum format_type {
};
struct printf_spec {
- enum format_type type;
- int flags; /* flags to number() */
- int field_width; /* width of output field */
- int base;
- int precision; /* # of digits/chars */
- int qualifier;
+ u16 type;
+ s16 field_width; /* width of output field */
+ u8 flags; /* flags to number() */
+ u8 base;
+ s8 precision; /* # of digits/chars */
+ u8 qualifier;
};
static char *number(char *buf, char *end, unsigned long long num,
@@ -1333,7 +1333,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
break;
case FORMAT_TYPE_NRCHARS: {
- int qualifier = spec.qualifier;
+ u8 qualifier = spec.qualifier;
if (qualifier == 'l') {
long *ip = va_arg(args, long *);
@@ -1619,7 +1619,7 @@ do { \
case FORMAT_TYPE_NRCHARS: {
/* skip %n 's argument */
- int qualifier = spec.qualifier;
+ u8 qualifier = spec.qualifier;
void *skip_arg;
if (qualifier == 'l')
skip_arg = va_arg(args, long *);
@@ -1885,7 +1885,9 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
char *next;
char digit;
int num = 0;
- int qualifier, base, field_width;
+ u8 qualifier;
+ u8 base;
+ s16 field_width;
bool is_sign;
while (*fmt && *str) {
@@ -1963,7 +1965,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
{
char *s = (char *)va_arg(args, char *);
if (field_width == -1)
- field_width = INT_MAX;
+ field_width = SHORT_MAX;
/* first, skip leading white space in buffer */
str = skip_spaces(str);
^ permalink raw reply related
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-06 23:46 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <1267918554.849.89.camel@Joe-Laptop.home>
On Sat, 6 Mar 2010, Joe Perches wrote:
>
> I believe 32 bits isn't possible.
>
> type:6
> flags:8
> width:8
> base:6
> precision:6
> qualifier:8
type: 5
flags: 7
width: 5 (make it 6 bits and signed)
base: 5
precision: 5 (make it 6 bits, and signed)
qualifier: 3
Now, admittedly, right now 'qualifier' really has to be 8, but that's just
because we use an ASCII character for it. But I don't think there are more
than 8 actual legal qualifiers. And 'base' could be just 2 bits, since
there are only a couple of legal bases, but I left room for them all
anyway.
And the other really should fit in those numbers unchanged, I think.
Linus
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-06 23:48 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061540470.23804@localhost.localdomain>
On Sat, 6 Mar 2010, Linus Torvalds wrote:
>
> width: 5 (make it 6 bits and signed)
> precision: 5 (make it 6 bits, and signed)
Oh, actually, no, we do that whole left-justification with the flags, we
don't need that signed stuff. We do need one special value for the "no
width, no precision", though.
Linus
^ permalink raw reply
* Re: Help On function to get IPV6 address of an interface in kernel
From: Mark Smith @ 2010-03-06 23:23 UTC (permalink / raw)
To: Venkata Mohan Reddy; +Cc: Brian Haley, netdev
In-Reply-To: <20100305154932.GA5139@mohan-laptop>
On Fri, 5 Mar 2010 21:19:34 +0530
Venkata Mohan Reddy <mohanreddykv@gmail.com> wrote:
> Thanks a lot Brain. ipv6_get_lladdr() helped me. Actually I want to get site-local or univeral(global) address. Though there is no direct function to get theseaddresses, ipv6_get_lladdr() shows how we can get the other addresses too.
>
Site-locals are deprecated (http://www.rfc-editor.org/rfc/rfc3879.txt),
so I wouldn't think you'll ever find a site-local specific address
function. If you do find one, I'd be fairly sure it'd go away.
> Thanks,
> Mohan Reddy
>
> On Thu, Mar 04, 2010 at 11:17:39AM -0500, Brian Haley wrote:
> > Mohan Reddy wrote:
> > > Hi,
> > >
> > > Is there a function or a way in kernel to get an interface ipv6 address if interface name or net_device object is known? I searched in the kernel i got a function ipv6_get_ifaddr(). But it is expecting an ipv6 address as a parameter.
> >
> > What type of IPv6 address? For link-locals you can use ipv6_get_lladdr(), but
> > for others ipv6_dev_get_saddr() is more comprehensive, but you'll need a
> > destination address. There's lots of reference code in addrconf.c to do
> > other things.
> >
> > -Brian
> --
> 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: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Joe Perches @ 2010-03-06 23:57 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061547490.23804@localhost.localdomain>
On Sat, 2010-03-06 at 15:48 -0800, Linus Torvalds wrote:
> On Sat, 6 Mar 2010, Linus Torvalds wrote:
> > width: 5 (make it 6 bits and signed)
> > precision: 5 (make it 6 bits, and signed)
> Oh, actually, no, we do that whole left-justification with the flags, we
> don't need that signed stuff. We do need one special value for the "no
> width, no precision", though.
What I posted is pretty simple.
I think the added complexity isn't worth it.
If you or anyone else does, all yours.
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-06 23:58 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061547490.23804@localhost.localdomain>
On Sat, 6 Mar 2010, Linus Torvalds wrote:
>
>
> On Sat, 6 Mar 2010, Linus Torvalds wrote:
> >
> > width: 5 (make it 6 bits and signed)
> > precision: 5 (make it 6 bits, and signed)
>
> Oh, actually, no, we do that whole left-justification with the flags, we
> don't need that signed stuff. We do need one special value for the "no
> width, no precision", though.
Btw, yeah, we do have a couple of "%.100s" etc, but we could fix those if
we really wanted to do this.
And no, I guess we don't really need to make it 32-bit. Fitting in 64 bits
would already be a big improvement over what we have now.
Linus
^ permalink raw reply
* [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes
From: Joe Perches @ 2010-03-07 1:10 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <alpine.LFD.2.00.1003061556420.23804@localhost.localdomain>
On Sat, 2010-03-06 at 15:58 -0800, Linus Torvalds wrote:
> And no, I guess we don't really need to make it 32-bit. Fitting in 64 bits
> would already be a big improvement over what we have now.
Reducing the size of struct printf_spec is a good thing
because multiple instances are commonly passed on stack.
It's possible for type to be u8 and field_width to be s8,
but this is likely small enough for now.
Signed-off-by: Joe Perches <joe@perches.com>
---
lib/vsprintf.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index af4aaa6..fdee7f7 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -408,12 +408,12 @@ enum format_type {
};
struct printf_spec {
- enum format_type type;
- int flags; /* flags to number() */
- int field_width; /* width of output field */
- int base;
- int precision; /* # of digits/chars */
- int qualifier;
+ u16 type;
+ s16 field_width; /* width of output field */
+ u8 flags; /* flags to number() */
+ u8 base;
+ s8 precision; /* # of digits/chars */
+ u8 qualifier;
};
static char *number(char *buf, char *end, unsigned long long num,
@@ -1333,7 +1333,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
break;
case FORMAT_TYPE_NRCHARS: {
- int qualifier = spec.qualifier;
+ u8 qualifier = spec.qualifier;
if (qualifier == 'l') {
long *ip = va_arg(args, long *);
@@ -1619,7 +1619,7 @@ do { \
case FORMAT_TYPE_NRCHARS: {
/* skip %n 's argument */
- int qualifier = spec.qualifier;
+ u8 qualifier = spec.qualifier;
void *skip_arg;
if (qualifier == 'l')
skip_arg = va_arg(args, long *);
@@ -1885,7 +1885,9 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
char *next;
char digit;
int num = 0;
- int qualifier, base, field_width;
+ u8 qualifier;
+ u8 base;
+ s16 field_width;
bool is_sign;
while (*fmt && *str) {
@@ -1963,7 +1965,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
{
char *s = (char *)va_arg(args, char *);
if (field_width == -1)
- field_width = INT_MAX;
+ field_width = SHORT_MAX;
/* first, skip leading white space in buffer */
str = skip_spaces(str);
^ permalink raw reply related
* Re: [net-next-2.6 PATCH v2] net: consolidate netif_needs_gso() checks
From: Herbert Xu @ 2010-03-07 1:43 UTC (permalink / raw)
To: John Fastabend
Cc: David Miller, Kirsher, Jeffrey T, netdev@vger.kernel.org,
gospo@redhat.com
In-Reply-To: <4B92ACB6.1090909@intel.com>
On Sat, Mar 06, 2010 at 11:27:50AM -0800, John Fastabend wrote:
>
> It looks like dev_gso_segment() could be used to "Verify header
> integrity only" according to the comment? If this is true I think the
> logic should probably be
>
> if (netif_needs_gso(dev, skb)) {
> if (unlikely(dev_gso_segment(skb)))
> goto out_kfree_skb;
> if (skb->next)
> goto gso;
> } do your thing
>
>
>
> That way we linearize the skb if necessary in the case were
> dev_gso_segment() only verifies the header and does not return a list of
> segments.
If we needed to linearise the skb then dev_gso_segment should
perform the segmentation. Is there a case where it doesn't?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes
From: Linus Torvalds @ 2010-03-07 2:03 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas
In-Reply-To: <1267924214.1937.12.camel@Joe-Laptop.home>
On Sat, 6 Mar 2010, Joe Perches wrote:
>
> Reducing the size of struct printf_spec is a good thing
> because multiple instances are commonly passed on stack.
Sadly, this is not enough.
the 'pointer()' function has a 200+ byte stack footprint on x86-64. And
vsnprintf itself is about 100+ bytes. So that stack depth is way bigger
than I would have expected.
I'm not sure _why_ that stack footprint for 'pointer()' is so big, but I
bet it's due to some simple inlining, and gcc (once more) sucking at it
and not being able to combine stack frames. It's a damn shame.
I suspect it's mac_address_string() having 20-odd bytes of temporary data,
ip6_compressed_string() having some more, ip6_addr_string() having 50
bytes, uuid_string() adding thirty-odd bytes etc. Inline them all, suck up
merging stack slots, and 200 bytes is easy.
So no, I don't think we can do the recursion as things stand. I've applied
your cleanup patch, along with the two from Bjorn Helgaas (which he did
for his pnp set), but they don't help this fundamental problem.
A few noinlines might be appropriate. As would a good gcc cluestick about
inlining and stack usage. The latter is unlikely to materialize, I guess.
Linus
^ permalink raw reply
* Re: [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes
From: Linus Torvalds @ 2010-03-07 2:24 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas
In-Reply-To: <alpine.LFD.2.00.1003061755400.23804@localhost.localdomain>
On Sat, 6 Mar 2010, Linus Torvalds wrote:
> the 'pointer()' function has a 200+ byte stack footprint on x86-64. And
> vsnprintf itself is about 100+ bytes. So that stack depth is way bigger
> than I would have expected.
>
> I'm not sure _why_ that stack footprint for 'pointer()' is so big, but I
> bet it's due to some simple inlining, and gcc (once more) sucking at it
> and not being able to combine stack frames. It's a damn shame.
Yeah, a few noinline's gets 'pointer()' to just save registers on the
stack, no need for any extra buffers (which then is ok for your recursion
case - the other subfunctions it can call have their own buffers, of
course, but they won't be in the recursive call-path except at the leaf.
vsnprintf() itself seems less obviously fixable. I'm not sure wht gcc
decides it needs 88 bytes of temp-space there.
Linus
^ permalink raw reply
* [PATCH] vsprintf.c: Use noinline_for_stack
From: Joe Perches @ 2010-03-07 2:33 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas
In-Reply-To: <alpine.LFD.2.00.1003061755400.23804@localhost.localdomain>
On Sat, 2010-03-06 at 18:03 -0800, Linus Torvalds wrote:
> A few noinlines might be appropriate.
Mark static functions with noinline_for_stack
Signed-off-by: Joe Perches <joe@perches.com>
---
lib/vsprintf.c | 67 +++++++++++++++++++++++++++++++++++---------------------
1 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 0d461c7..e9335a8 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -266,7 +266,8 @@ int strict_strtoll(const char *cp, unsigned int base, long long *res)
}
EXPORT_SYMBOL(strict_strtoll);
-static int skip_atoi(const char **s)
+static noinline_for_stack
+int skip_atoi(const char **s)
{
int i = 0;
@@ -286,7 +287,8 @@ static int skip_atoi(const char **s)
/* Formats correctly any integer in [0,99999].
* Outputs from one to five digits depending on input.
* On i386 gcc 4.1.2 -O2: ~250 bytes of code. */
-static char *put_dec_trunc(char *buf, unsigned q)
+static noinline_for_stack
+char *put_dec_trunc(char *buf, unsigned q)
{
unsigned d3, d2, d1, d0;
d1 = (q>>4) & 0xf;
@@ -323,7 +325,8 @@ static char *put_dec_trunc(char *buf, unsigned q)
return buf;
}
/* Same with if's removed. Always emits five digits */
-static char *put_dec_full(char *buf, unsigned q)
+static noinline_for_stack
+char *put_dec_full(char *buf, unsigned q)
{
/* BTW, if q is in [0,9999], 8-bit ints will be enough, */
/* but anyway, gcc produces better code with full-sized ints */
@@ -365,7 +368,8 @@ static char *put_dec_full(char *buf, unsigned q)
return buf;
}
/* No inlining helps gcc to use registers better */
-static noinline char *put_dec(char *buf, unsigned long long num)
+static noinline_for_stack
+char *put_dec(char *buf, unsigned long long num)
{
while (1) {
unsigned rem;
@@ -416,8 +420,9 @@ struct printf_spec {
u8 qualifier;
};
-static char *number(char *buf, char *end, unsigned long long num,
- struct printf_spec spec)
+static noinline_for_stack
+char *number(char *buf, char *end, unsigned long long num,
+ struct printf_spec spec)
{
/* we are called with base 8, 10 or 16, only, thus don't need "G..." */
static const char digits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */
@@ -536,7 +541,8 @@ static char *number(char *buf, char *end, unsigned long long num,
return buf;
}
-static char *string(char *buf, char *end, const char *s, struct printf_spec spec)
+static noinline_for_stack
+char *string(char *buf, char *end, const char *s, struct printf_spec spec)
{
int len, i;
@@ -566,8 +572,9 @@ static char *string(char *buf, char *end, const char *s, struct printf_spec spec
return buf;
}
-static char *symbol_string(char *buf, char *end, void *ptr,
- struct printf_spec spec, char ext)
+static noinline_for_stack
+char *symbol_string(char *buf, char *end, void *ptr,
+ struct printf_spec spec, char ext)
{
unsigned long value = (unsigned long) ptr;
#ifdef CONFIG_KALLSYMS
@@ -587,8 +594,9 @@ static char *symbol_string(char *buf, char *end, void *ptr,
#endif
}
-static char *resource_string(char *buf, char *end, struct resource *res,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *resource_string(char *buf, char *end, struct resource *res,
+ struct printf_spec spec, const char *fmt)
{
#ifndef IO_RSRC_PRINTK_SIZE
#define IO_RSRC_PRINTK_SIZE 6
@@ -678,8 +686,9 @@ static char *resource_string(char *buf, char *end, struct resource *res,
return string(buf, end, sym, spec);
}
-static char *mac_address_string(char *buf, char *end, u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *mac_address_string(char *buf, char *end, u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char mac_addr[sizeof("xx:xx:xx:xx:xx:xx")];
char *p = mac_addr;
@@ -702,7 +711,8 @@ static char *mac_address_string(char *buf, char *end, u8 *addr,
return string(buf, end, mac_addr, spec);
}
-static char *ip4_string(char *p, const u8 *addr, const char *fmt)
+static noinline_for_stack
+char *ip4_string(char *p, const u8 *addr, const char *fmt)
{
int i;
bool leading_zeros = (fmt[0] == 'i');
@@ -751,7 +761,8 @@ static char *ip4_string(char *p, const u8 *addr, const char *fmt)
return p;
}
-static char *ip6_compressed_string(char *p, const char *addr)
+static noinline_for_stack
+char *ip6_compressed_string(char *p, const char *addr)
{
int i, j, range;
unsigned char zerolength[8];
@@ -831,7 +842,8 @@ static char *ip6_compressed_string(char *p, const char *addr)
return p;
}
-static char *ip6_string(char *p, const char *addr, const char *fmt)
+static noinline_for_stack
+char *ip6_string(char *p, const char *addr, const char *fmt)
{
int i;
@@ -846,8 +858,9 @@ static char *ip6_string(char *p, const char *addr, const char *fmt)
return p;
}
-static char *ip6_addr_string(char *buf, char *end, const u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *ip6_addr_string(char *buf, char *end, const u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char ip6_addr[sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:255.255.255.255")];
@@ -859,8 +872,9 @@ static char *ip6_addr_string(char *buf, char *end, const u8 *addr,
return string(buf, end, ip6_addr, spec);
}
-static char *ip4_addr_string(char *buf, char *end, const u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *ip4_addr_string(char *buf, char *end, const u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char ip4_addr[sizeof("255.255.255.255")];
@@ -869,8 +883,9 @@ static char *ip4_addr_string(char *buf, char *end, const u8 *addr,
return string(buf, end, ip4_addr, spec);
}
-static char *uuid_string(char *buf, char *end, const u8 *addr,
- struct printf_spec spec, const char *fmt)
+static noinline_for_stack
+char *uuid_string(char *buf, char *end, const u8 *addr,
+ struct printf_spec spec, const char *fmt)
{
char uuid[sizeof("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx")];
char *p = uuid;
@@ -958,8 +973,9 @@ static char *uuid_string(char *buf, char *end, const u8 *addr,
* function pointers are really function descriptors, which contain a
* pointer to the real address.
*/
-static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
- struct printf_spec spec)
+static noinline_for_stack
+char *pointer(const char *fmt, char *buf, char *end, void *ptr,
+ struct printf_spec spec)
{
if (!ptr)
return string(buf, end, "(null)", spec);
@@ -1028,7 +1044,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
* @precision: precision of a number
* @qualifier: qualifier of a number (long, size_t, ...)
*/
-static int format_decode(const char *fmt, struct printf_spec *spec)
+static noinline_for_stack
+int format_decode(const char *fmt, struct printf_spec *spec)
{
const char *start = fmt;
^ permalink raw reply related
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Herbert Xu @ 2010-03-07 2:45 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <20100306190000.GA24445@linux.vnet.ibm.com>
On Sat, Mar 06, 2010 at 11:00:00AM -0800, Paul E. McKenney wrote:
>
> > Hmmm... rcu_barrier() definitely does -not- imply rcu_barrier_bh(),
> > because there are separate sets of callbacks whose execution can
> > be throttled separately. So, while you would expect RCU-bh grace
> > periods to complete more quickly, if there was a large number of
> > RCU-bh callbacks on a given CPU but very few RCU callbacks, it might
> > well take longer for the RCU-bh callbacks to be invoked.
> >
> > With TREE_PREEMPT_RCU, if there were no RCU readers but one long-running
> > RCU-bh reader, then synchronize_rcu_bh() could return before
> > synchronize_rcu() does.
OK, then we definitely do have some issues under net/ with respect
to the two types of RCU usage. As you can see, we use the RCU-BH
variant on the read-side in various places, and call_rcu_bh on the
write-side too, but we only ever use the non-BH version of the
functions rcu_barrier and synchronize_rcu.
Now there is a possibility that the places where we use synchronize
and rcu_barrier don't really care about the BH variant, but an
audit wouldn't hurt.
> You really are talking about code like the following, correct?
>
> rcu_read_lock();
> p = rcu_dereference(global_p);
> do_something_with(p);
> rcu_read_unlock();
>
> . . .
>
> rcu_read_lock_bh();
> p = rcu_dereference(global_p);
> do_something_else_with(p);
> rcu_read_unlock_bh();
>
> . . .
>
> spin_lock(&my_lock);
> p = global_p;
> rcu_assign_pointer(global_p, NULL);
> synchronize_rcu(); /* BUG -- also need synchronize_rcu_bh(). */
> kfree(p);
> spin_unlock(&my_lock);
>
> In other words, different readers traversing the same data structure
> under different flavors of RCU protection, but then using only one
> flavor of RCU grace period during the update?
We usually don't use synchronize_rcu/rcu_barrier on the update side,
but rather they are used in the tear-down process.
But otherwise yes this is exactly my concern.
Note that we may have a problem on the update side too if we used
the wrong call_rcu variant, but it would require a thorough audit
to reveal those.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Paul E. McKenney @ 2010-03-07 3:11 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev, Stephen Hemminger, arnd
In-Reply-To: <20100307024500.GA20126@gondor.apana.org.au>
On Sun, Mar 07, 2010 at 10:45:00AM +0800, Herbert Xu wrote:
> On Sat, Mar 06, 2010 at 11:00:00AM -0800, Paul E. McKenney wrote:
> >
> > > Hmmm... rcu_barrier() definitely does -not- imply rcu_barrier_bh(),
> > > because there are separate sets of callbacks whose execution can
> > > be throttled separately. So, while you would expect RCU-bh grace
> > > periods to complete more quickly, if there was a large number of
> > > RCU-bh callbacks on a given CPU but very few RCU callbacks, it might
> > > well take longer for the RCU-bh callbacks to be invoked.
> > >
> > > With TREE_PREEMPT_RCU, if there were no RCU readers but one long-running
> > > RCU-bh reader, then synchronize_rcu_bh() could return before
> > > synchronize_rcu() does.
>
> OK, then we definitely do have some issues under net/ with respect
> to the two types of RCU usage. As you can see, we use the RCU-BH
> variant on the read-side in various places, and call_rcu_bh on the
> write-side too, but we only ever use the non-BH version of the
> functions rcu_barrier and synchronize_rcu.
>
> Now there is a possibility that the places where we use synchronize
> and rcu_barrier don't really care about the BH variant, but an
> audit wouldn't hurt.
>
> > You really are talking about code like the following, correct?
> >
> > rcu_read_lock();
> > p = rcu_dereference(global_p);
> > do_something_with(p);
> > rcu_read_unlock();
> >
> > . . .
> >
> > rcu_read_lock_bh();
> > p = rcu_dereference(global_p);
> > do_something_else_with(p);
> > rcu_read_unlock_bh();
> >
> > . . .
> >
> > spin_lock(&my_lock);
> > p = global_p;
> > rcu_assign_pointer(global_p, NULL);
> > synchronize_rcu(); /* BUG -- also need synchronize_rcu_bh(). */
> > kfree(p);
> > spin_unlock(&my_lock);
> >
> > In other words, different readers traversing the same data structure
> > under different flavors of RCU protection, but then using only one
> > flavor of RCU grace period during the update?
>
> We usually don't use synchronize_rcu/rcu_barrier on the update side,
> but rather they are used in the tear-down process.
>
> But otherwise yes this is exactly my concern.
>
> Note that we may have a problem on the update side too if we used
> the wrong call_rcu variant, but it would require a thorough audit
> to reveal those.
OK, just re-checked your patch, and it looks OK.
Also adding Arnd to CC.
Arnd, would it be reasonable to extend your RCU-sparse changes to have
four different pointer namespaces, one for each flavor of RCU? (RCU,
RCU-bh, RCU-sched, and SRCU)? Always a fan of making the computer do
the auditing where reasonable. ;-)
This could potentially catch the mismatched call_rcu()s, at least if the
rcu_head could be labeled.
Other thoughts?
Thanx, Paul
^ permalink raw reply
* Re: [RFC v2 00/10] snet: Security for NETwork syscalls
From: Tetsuo Handa @ 2010-03-07 5:45 UTC (permalink / raw)
To: sam
Cc: linux-kernel, netdev, netfilter-devel, hadi, kaber, zbr, nhorman,
root, linux-security-module
In-Reply-To: <m2vdd9p9r7.fsf@ssh.synack.fr>
Hello.
Samir Bellabes wrote:
> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
>
> > Regarding [RFC v2 09/10] snet: introduce snet_ticket
> > +enum snet_verdict snet_ticket_check(struct snet_info *info)
> > +{
> > + struct snet_ticket *st = NULL;
> > + unsigned int h = 0, verdict = SNET_VERDICT_NONE;
> > + struct list_head *l = NULL;
> > + struct snet_task_security *tsec = NULL;
> > +
> > + if (snet_ticket_mode == SNET_TICKET_OFF)
> > + goto out;
> > +
> > + tsec = (struct snet_task_security*) current_security();
> > +
> > + h = jhash_2words(info->syscall, info->protocol, 0) % HSIZE;
> > + l = &tsec->hash[h];
> > +
> > + read_lock_bh(&tsec->lock);
> >
> > Credentials are allocated for copy-on-write basis.
> > Sharing "tsec" among multiple "struct task_struct" is what you intended?
>
> No, there is no shared "tsec".
> snet_ticket_check() is called from the process context. So "tsec" is
> a pointer to the "void *security" pointer from its own "struct
> task_struct".
>
Until 2.6.28:
"void *security" is directory attached to "struct task_struct".
copy_process() calls security_task_alloc().
Therefore, task1->security != task2->security is guaranteed as long as you do
task->security = kmalloc();
at security_task_alloc().
Since 2.6.29:
"void *security" is attached to "struct cred", and "struct cred *" is
attached to "struct task_struct". copy_process() calls copy_creds() and
prepare_creds() calls security_prepare_creds(). But copy_creds() does not
call prepare_creds() for clone(CLONE_THREAD) case.
Therefore, task1->cred->security != task2->cred->security is not guaranteed
even if you do
cred->security = kmalloc();
at security_prepare_creds().
> every task_struct have a "tsec" allocated to its "void *security"
> pointer.
You meant to have assigned "void *security" dedicated to "struct task_struct".
But "void *security" is no longer directly attached to "struct task_struct".
I couldn't find code that checks whether "current->cred" is used by only
current thread or not. "current->cred" being used by only current thread is
a requirement for having a "tsec" allocated to every "struct task_struct".
Your code will share "tsec" among multiple threads if a process created
threads using clone(CLONE_THREAD | CLONE_SIGHAND | CLONE_VM). Each thread has
its own "struct task_struct" but they share "cred->security".
Sharing "tsec" among multiple threads is what you intended?
Regards.
^ permalink raw reply
* Re: [RFC v2 00/10] snet: Security for NETwork syscalls
From: Tetsuo Handa @ 2010-03-07 5:47 UTC (permalink / raw)
To: sam
Cc: linux-kernel, netdev, netfilter-devel, hadi, kaber, zbr, nhorman,
root, linux-security-module, serue
In-Reply-To: <m2zl2lpa4d.fsf@ssh.synack.fr>
Samir Bellabes wrote:
> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
>
> > Regarding [RFC v2 02/10] Revert "lsm: Remove the socket_post_accept() hook"
> > @@ -1538,6 +1538,8 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr __user *, upeer_sockaddr,
> > fd_install(newfd, newfile);
> > err = newfd;
> >
> > + security_socket_post_accept(sock, newsock);
> > +
> > out_put:
> > fput_light(sock->file, fput_needed);
> > out:
> >
> > Please move security_socket_post_accept() to before fd_install().
> > Otherwise, other threads which share fd tables can use
> > security-informations-not-yet-updated accept()ed sockets.
>
> Tetsuo, what about this patch ?
>
Looks OK to me. Thanks.
^ permalink raw reply
* [PATCH net-2.6] netfilter: ebt_ip6: Use ipv6_masked_addr_cmp().
From: YOSHIFUJI Hideaki @ 2010-03-07 8:39 UTC (permalink / raw)
To: bart.de.schuymer, kaber, davem
Cc: ebtables-devel, netfilter-devel, netdev, yoshfuji
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
net/bridge/netfilter/ebt_ip6.c | 18 ++++--------------
1 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/net/bridge/netfilter/ebt_ip6.c b/net/bridge/netfilter/ebt_ip6.c
index bbf2534..4644cc9 100644
--- a/net/bridge/netfilter/ebt_ip6.c
+++ b/net/bridge/netfilter/ebt_ip6.c
@@ -35,8 +35,6 @@ ebt_ip6_mt(const struct sk_buff *skb, const struct xt_match_param *par)
struct ipv6hdr _ip6h;
const struct tcpudphdr *pptr;
struct tcpudphdr _ports;
- struct in6_addr tmp_addr;
- int i;
ih6 = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h);
if (ih6 == NULL)
@@ -44,18 +42,10 @@ ebt_ip6_mt(const struct sk_buff *skb, const struct xt_match_param *par)
if (info->bitmask & EBT_IP6_TCLASS &&
FWINV(info->tclass != ipv6_get_dsfield(ih6), EBT_IP6_TCLASS))
return false;
- for (i = 0; i < 4; i++)
- tmp_addr.in6_u.u6_addr32[i] = ih6->saddr.in6_u.u6_addr32[i] &
- info->smsk.in6_u.u6_addr32[i];
- if (info->bitmask & EBT_IP6_SOURCE &&
- FWINV((ipv6_addr_cmp(&tmp_addr, &info->saddr) != 0),
- EBT_IP6_SOURCE))
- return false;
- for (i = 0; i < 4; i++)
- tmp_addr.in6_u.u6_addr32[i] = ih6->daddr.in6_u.u6_addr32[i] &
- info->dmsk.in6_u.u6_addr32[i];
- if (info->bitmask & EBT_IP6_DEST &&
- FWINV((ipv6_addr_cmp(&tmp_addr, &info->daddr) != 0), EBT_IP6_DEST))
+ if (FWINV(ipv6_masked_addr_cmp(&ih6->saddr, &info->smsk,
+ &info->saddr), EBT_IP6_SOURCE) ||
+ FWINV(ipv6_masked_addr_cmp(&ih6->daddr, &info->dmsk,
+ &info->daddr), EBT_IP6_DEST))
return false;
if (info->bitmask & EBT_IP6_PROTO) {
uint8_t nexthdr = ih6->nexthdr;
--
1.5.6.5
^ permalink raw reply related
* [PATCH net-2.6] netfilter: Remove stale declaration for ip6_masked_addrcmp().
From: YOSHIFUJI Hideaki @ 2010-03-07 8:39 UTC (permalink / raw)
To: kaber, davem; +Cc: netfilter-devel, netdev, yoshfuji
Commit f2ffd9ee... ("[NETFILTER]: Move ip6_masked_addrcmp to
include/net/ipv6.h") replaced ip6_masked_addrcmp() with
ipv6_masked_addr_cmp(). Function definition went away.
Let's remove its declaration as well in header file.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/linux/netfilter_ipv6/ip6_tables.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/include/linux/netfilter_ipv6/ip6_tables.h b/include/linux/netfilter_ipv6/ip6_tables.h
index e5ba03d..18442ff 100644
--- a/include/linux/netfilter_ipv6/ip6_tables.h
+++ b/include/linux/netfilter_ipv6/ip6_tables.h
@@ -316,10 +316,6 @@ extern int ip6t_ext_hdr(u8 nexthdr);
extern int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
int target, unsigned short *fragoff);
-extern int ip6_masked_addrcmp(const struct in6_addr *addr1,
- const struct in6_addr *mask,
- const struct in6_addr *addr2);
-
#define IP6T_ALIGN(s) XT_ALIGN(s)
#ifdef CONFIG_COMPAT
--
1.5.6.5
^ permalink raw reply related
* Re: drivers/net/mac8390.c: Remove useless memcpy casting
From: Geert Uytterhoeven @ 2010-03-07 9:19 UTC (permalink / raw)
To: Joe Perches
Cc: David S. Miller, netdev, Linux Kernel Mailing List, Linux/m68k
In-Reply-To: <201003022001.o22K16vJ011337@hera.kernel.org>
On Tue, Mar 2, 2010 at 21:01, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> Gitweb: http://git.kernel.org/linus/5c7fffd0e3b57cb63f50bbd710868f012d67654f
> Commit: 5c7fffd0e3b57cb63f50bbd710868f012d67654f
> Parent: 35076402a9936fa8a73b57a1f97fecbeceeec34a
> Author: Joe Perches <joe@perches.com>
> AuthorDate: Mon Jan 4 11:53:00 2010 +0000
> Committer: David S. Miller <davem@davemloft.net>
> CommitDate: Wed Jan 6 20:44:02 2010 -0800
>
> drivers/net/mac8390.c: Remove useless memcpy casting
>
> Signed-off-by: Joe Perches <joe@perches.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> drivers/net/mac8390.c | 19 ++++++++++---------
> 1 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/mac8390.c b/drivers/net/mac8390.c
> index 6b6f375..517cee4 100644
> --- a/drivers/net/mac8390.c
> +++ b/drivers/net/mac8390.c
> @@ -237,14 +237,14 @@ static enum mac8390_access __init mac8390_testio(volatile unsigned long membase)
As indicated by the prototype above, membase is not a pointer...
> unsigned long outdata = 0xA5A0B5B0;
> unsigned long indata = 0x00000000;
> /* Try writing 32 bits */
> - memcpy((char *)membase, (char *)&outdata, 4);
> + memcpy(membase, &outdata, 4);
> /* Now compare them */
> if (memcmp((char *)&outdata, (char *)membase, 4) == 0)
> return ACCESS_32;
> /* Write 16 bit output */
> - word_memcpy_tocard((char *)membase, (char *)&outdata, 4);
> + word_memcpy_tocard(membase, &outdata, 4);
> /* Now read it back */
> - word_memcpy_fromcard((char *)&indata, (char *)membase, 4);
> + word_memcpy_fromcard(&indata, membase, 4);
> if (outdata == indata)
> return ACCESS_16;
> return ACCESS_UNKNOWN;
... hence you introduced 3 compiler warnings:
drivers/net/mac8390.c:249: warning: passing argument 1 of
'__builtin_memcpy' makes pointer from integer without a cast
drivers/net/mac8390.c:254: warning: passing argument 1 of
'word_memcpy_tocard' makes pointer from integer without a cast
drivers/net/mac8390.c:256: warning: passing argument 2 of
'word_memcpy_fromcard' makes pointer from integer without a cast
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
^ permalink raw reply
* [PATCH net-next-2.6] ipv6: Optmize translation between IPV6_PREFER_SRC_xxx and RT6_LOOKUP_F_xxx.
From: YOSHIFUJI Hideaki @ 2010-03-07 10:14 UTC (permalink / raw)
To: davem; +Cc: yoshfuji, netdev
IPV6_PREFER_SRC_xxx definitions:
| #define IPV6_PREFER_SRC_TMP 0x0001
| #define IPV6_PREFER_SRC_PUBLIC 0x0002
| #define IPV6_PREFER_SRC_COA 0x0004
RT6_LOOKUP_F_xxx definitions:
| #define RT6_LOOKUP_F_SRCPREF_TMP 0x00000008
| #define RT6_LOOKUP_F_SRCPREF_PUBLIC 0x00000010
| #define RT6_LOOKUP_F_SRCPREF_COA 0x00000020
So, we can translate between these two groups by shift operation
instead of multiple 'if's.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/net/ip6_route.h | 18 ++++++++++++++++++
net/ipv6/fib6_rules.c | 11 ++---------
net/ipv6/route.c | 11 ++---------
3 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 4a808de..68f6783 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -37,6 +37,24 @@ struct route_info {
#define RT6_LOOKUP_F_SRCPREF_PUBLIC 0x00000010
#define RT6_LOOKUP_F_SRCPREF_COA 0x00000020
+/*
+ * rt6_srcprefs2flags() and rt6_flags2srcprefs() translate
+ * between IPV6_ADDR_PREFERENCES socket option values
+ * IPV6_PREFER_SRC_TMP = 0x1
+ * IPV6_PREFER_SRC_PUBLIC = 0x2
+ * IPV6_PREFER_SRC_COA = 0x4
+ * and above RT6_LOOKUP_F_SRCPREF_xxx flags.
+ */
+static inline int rt6_srcprefs2flags(unsigned int srcprefs)
+{
+ /* No need to bitmask because srcprefs have only 3 bits. */
+ return srcprefs << 3;
+}
+
+static inline unsigned int rt6_flags2srcprefs(int flags)
+{
+ return (flags >> 3) & 7;
+}
extern void ip6_route_input(struct sk_buff *skb);
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 551882b..5e463c4 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -84,18 +84,11 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
if ((rule->flags & FIB_RULE_FIND_SADDR) &&
r->src.plen && !(flags & RT6_LOOKUP_F_HAS_SADDR)) {
struct in6_addr saddr;
- unsigned int srcprefs = 0;
-
- if (flags & RT6_LOOKUP_F_SRCPREF_TMP)
- srcprefs |= IPV6_PREFER_SRC_TMP;
- if (flags & RT6_LOOKUP_F_SRCPREF_PUBLIC)
- srcprefs |= IPV6_PREFER_SRC_PUBLIC;
- if (flags & RT6_LOOKUP_F_SRCPREF_COA)
- srcprefs |= IPV6_PREFER_SRC_COA;
if (ipv6_dev_get_saddr(net,
ip6_dst_idev(&rt->u.dst)->dev,
- &flp->fl6_dst, srcprefs,
+ &flp->fl6_dst,
+ rt6_flags2srcprefs(flags),
&saddr))
goto again;
if (!ipv6_prefix_equal(&saddr, &r->src.addr,
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b08879e..52cd3ef 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -819,15 +819,8 @@ struct dst_entry * ip6_route_output(struct net *net, struct sock *sk,
if (!ipv6_addr_any(&fl->fl6_src))
flags |= RT6_LOOKUP_F_HAS_SADDR;
- else if (sk) {
- unsigned int prefs = inet6_sk(sk)->srcprefs;
- if (prefs & IPV6_PREFER_SRC_TMP)
- flags |= RT6_LOOKUP_F_SRCPREF_TMP;
- if (prefs & IPV6_PREFER_SRC_PUBLIC)
- flags |= RT6_LOOKUP_F_SRCPREF_PUBLIC;
- if (prefs & IPV6_PREFER_SRC_COA)
- flags |= RT6_LOOKUP_F_SRCPREF_COA;
- }
+ else if (sk)
+ flags |= rt6_srcprefs2flags(inet6_sk(sk)->srcprefs);
return fib6_rule_lookup(net, fl, flags, ip6_pol_route_output);
}
--
1.5.6.5
^ permalink raw reply related
* inconsistent lock state
From: Sergey Senozhatsky @ 2010-03-07 10:21 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Ingo Molnar
[-- Attachment #1: Type: text/plain, Size: 5293 bytes --]
Hello,
Hardly reproducible.
/*
* 2.6.33. x86. ASUS f3jc
*/
[329645.010697] =================================
[329645.010699] [ INFO: inconsistent lock state ]
[329645.010703] 2.6.33-33-0-dbg #31
[329645.010705] ---------------------------------
[329645.010708] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[329645.010712] events/0/9 [HC0[0]:SC0[0]:HE1:SE1] takes:
[329645.010715] (&(&table->hash[i].lock)->rlock){+.?.-.}, at: [<c12c1892>] spin_lock+0x8/0xa
[329645.010729] {IN-SOFTIRQ-W} state was registered at:
[329645.010732] [<c105878f>] __lock_acquire+0x27e/0xb86
[329645.010739] [<c1059988>] lock_acquire+0xa1/0xb8
[329645.010744] [<c12f39b2>] _raw_spin_lock+0x28/0x58
[329645.010750] [<c12c1892>] spin_lock+0x8/0xa
[329645.010755] [<c12c2c8f>] T.958+0x3c/0x141
[329645.010759] [<c12c2f7a>] __udp4_lib_rcv+0x1e6/0x3cd
[329645.010764] [<c12c3173>] udp_rcv+0x12/0x14
[329645.010769] [<c12a559b>] ip_local_deliver_finish+0xc9/0x130
[329645.010774] [<c12a5663>] ip_local_deliver+0x61/0x66
[329645.010778] [<c12a51a5>] ip_rcv_finish+0x275/0x29d
[329645.010783] [<c12a539a>] ip_rcv+0x1cd/0x1ed
[329645.010786] [<c128858d>] netif_receive_skb+0x340/0x360
[329645.010791] [<fd1ab6f4>] rtl8169_rx_interrupt+0x2bf/0x37e [r8169]
[329645.010801] [<fd1adab8>] rtl8169_poll+0x29/0x15a [r8169]
[329645.010808] [<c1288b8c>] net_rx_action+0x95/0x1af
[329645.010812] [<c1037243>] __do_softirq+0xc6/0x187
[329645.010819] [<c103732f>] do_softirq+0x2b/0x43
[329645.010823] [<c10374bf>] irq_exit+0x38/0x75
[329645.010828] [<c1004116>] do_IRQ+0x88/0x9c
[329645.010833] [<c1002f35>] common_interrupt+0x35/0x3c
[329645.010837] [<c1275d0b>] cpuidle_idle_call+0x72/0xd3
[329645.010844] [<c1001c1b>] cpu_idle+0x92/0xbf
[329645.010848] [<c12e3f16>] rest_init+0x76/0x78
[329645.010853] [<c14c2868>] start_kernel+0x33c/0x341
[329645.010859] [<c14c2092>] i386_start_kernel+0x92/0x99
[329645.010864] irq event stamp: 157782307
[329645.010866] hardirqs last enabled at (157782307): [<c10b74b8>] kmem_cache_free+0x97/0xd6
[329645.010873] hardirqs last disabled at (157782306): [<c10b745a>] kmem_cache_free+0x39/0xd6
[329645.010878] softirqs last enabled at (157782304): [<c12928ef>] rcu_read_unlock_bh+0x1c/0x1e
[329645.010885] softirqs last disabled at (157782302): [<c129289b>] rcu_read_lock_bh+0x8/0x26
[329645.010892]
[329645.010893] other info that might help us debug this:
[329645.010896] 5 locks held by events/0/9:
[329645.010898] #0: (events){+.+.+.}, at: [<c1044f2f>] worker_thread+0x16a/0x27c
[329645.010908] #1: ((&(&tp->task)->work)){+.+...}, at: [<c1044f2f>] worker_thread+0x16a/0x27c
[329645.010915] #2: (rtnl_mutex){+.+.+.}, at: [<c12911dd>] rtnl_lock+0xf/0x11
[329645.010922] #3: (rcu_read_lock){.+.+..}, at: [<c1286a01>] rcu_read_lock+0x0/0x2b
[329645.010931] #4: (rcu_read_lock){.+.+..}, at: [<c12a4ed0>] rcu_read_lock+0x0/0x2b
[329645.010938]
[329645.010939] stack backtrace:
[329645.010942] Pid: 9, comm: events/0 Not tainted 2.6.33-33-0-dbg #31
[329645.010945] Call Trace:
[329645.010950] [<c12f18ec>] ? printk+0xf/0x11
[329645.010955] [<c1057800>] valid_state+0x12a/0x13d
[329645.010960] [<c1057904>] mark_lock+0xf1/0x1e2
[329645.010965] [<c1057ffa>] ? check_usage_backwards+0x0/0x6f
[329645.010970] [<c10587fd>] __lock_acquire+0x2ec/0xb86
[329645.010976] [<c100864f>] ? native_sched_clock+0x48/0x8d
[329645.010982] [<c104d24b>] ? sched_clock_local+0x17/0x11e
[329645.010987] [<c12c1892>] ? spin_lock+0x8/0xa
[329645.010992] [<c1059988>] lock_acquire+0xa1/0xb8
[329645.010997] [<c12c1892>] ? spin_lock+0x8/0xa
[329645.011002] [<c12f39b2>] _raw_spin_lock+0x28/0x58
[329645.011006] [<c12c1892>] ? spin_lock+0x8/0xa
[329645.011010] [<c12c1892>] spin_lock+0x8/0xa
[329645.011015] [<c12c2c8f>] T.958+0x3c/0x141
[329645.011020] [<c104d472>] ? sched_clock_cpu+0x120/0x128
[329645.011025] [<c100864f>] ? native_sched_clock+0x48/0x8d
[329645.011031] [<c1059088>] ? __lock_acquire+0xb77/0xb86
[329645.011037] [<c12a1345>] ? rcu_read_unlock+0x0/0x35
[329645.011042] [<c12a38c0>] ? ip_route_input+0x102/0xacb
[329645.011046] [<c1057831>] ? mark_lock+0x1e/0x1e2
[329645.011051] [<c12a4ed0>] ? rcu_read_lock+0x0/0x2b
[329645.011056] [<c12c2f7a>] __udp4_lib_rcv+0x1e6/0x3cd
[329645.011061] [<c12c3173>] udp_rcv+0x12/0x14
[329645.011065] [<c12a559b>] ip_local_deliver_finish+0xc9/0x130
[329645.011070] [<c12a5663>] ip_local_deliver+0x61/0x66
[329645.011074] [<c12a51a5>] ip_rcv_finish+0x275/0x29d
[329645.011078] [<c12a539a>] ip_rcv+0x1cd/0x1ed
[329645.011083] [<c128858d>] netif_receive_skb+0x340/0x360
[329645.011093] [<fd1ab6f4>] rtl8169_rx_interrupt+0x2bf/0x37e [r8169]
[329645.011100] [<fd1aba02>] rtl8169_reset_task+0x38/0xcd [r8169]
[329645.011105] [<c1044f71>] worker_thread+0x1ac/0x27c
[329645.011110] [<c1044f2f>] ? worker_thread+0x16a/0x27c
[329645.011116] [<fd1ab9ca>] ? rtl8169_reset_task+0x0/0xcd [r8169]
[329645.011123] [<c1048725>] ? autoremove_wake_function+0x0/0x2f
[329645.011128] [<c1044dc5>] ? worker_thread+0x0/0x27c
[329645.011132] [<c104838a>] kthread+0x6a/0x6f
[329645.011137] [<c1048320>] ? kthread+0x0/0x6f
[329645.011142] [<c1002f42>] kernel_thread_helper+0x6/0x1a
Sergey
[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox