* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Scott Wood @ 2011-08-09 18:17 UTC (permalink / raw)
To: Robin Holt
Cc: Marc Kleine-Budde, Wolfgang Grandegger, U Bhaskar-B22300, netdev,
socketcan-core, PPC list
In-Reply-To: <1312901031-29887-6-git-send-email-holt@sgi.com>
On 08/09/2011 09:43 AM, Robin Holt wrote:
> In working with the socketcan developers, we have come to the conclusion
> the fsl-flexcan device tree bindings need to be cleaned up.
> The driver does not depend upon any properties other than the required properties
> so we are removing the file.
That is not the criterion for whether something should be expresed in
the device tree. It's a description of the hardware, not a Linux driver
configuration file. If there are integration parameters that can not be
inferred from "this is FSL flexcan v1.0", they should be expressed in
the node.
Removing the binding altogether seems extreme as well -- we should have
bindings for all devices, even if there are no special properties.
> Additionally, the p1010*dts files are not
> following the standard for node naming in that they have a trailing -v1.0.
What "standard for node naming"? There's nothing wrong with putting a
block version number in the compatible string, and it looks like the
p1010 dts files were following the binding document in this regard. It
is common practice when the block version is publicly documented but
there's no register it can be read from at runtime.
-Scott
^ permalink raw reply
* Re: [PATCH] gianfar: reduce stack usage in gianfar_ethtool.c
From: Eric Dumazet @ 2011-08-09 18:24 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel, stufever, netdev, davem, Wang Shaoyan
In-Reply-To: <1312913545.11924.16.camel@Joe-Laptop>
Le mardi 09 août 2011 à 11:12 -0700, Joe Perches a écrit :
> On Tue, 2011-08-09 at 19:59 +0200, Eric Dumazet wrote:
> > Le mardi 09 août 2011 à 10:52 -0700, Joe Perches a écrit :
> > > On Tue, 2011-08-09 at 18:53 +0200, Eric Dumazet wrote:
> > > > Le mercredi 10 août 2011 à 00:39 +0800, stufever@gmail.com a écrit :
> > > []
> > > > > + if (!local_rqfpr || !local_rqfcr) {
> > > > > + pr_err("Out of memory\n");
> > > > Please remove this pr_err(), kmalloc() will complain already.
> > > Always?
> > > I know there's a trace option, but is it always on?
> > Yes, unless caller added ___GFP_NOWARN in gfp :
> > ptr = kmalloc(size, GFP_KERNEL | ___GFP_NOWARN);
>
> Isn't that true only for slub?
>
nope, this is page allocation, so common to slab/slub/...
> Do you know where this get emitted?
> I looked cursorily but I don't see it.
>
This is a bit beyond this thread
Look at mm/page_alloc.c
void warn_alloc_failed(gfp_t gfp_mask, int order, const char *fmt, ...)
^ permalink raw reply
* Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits
From: Tom Herbert @ 2011-08-09 18:28 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Eric Dumazet, David Miller, netdev
In-Reply-To: <20110809075454.5331dcac@nehalam.ftrdhcpuser.net>
> Ok. then we need to fix all NAPI drivers at once, and get more
> drivers converted over to NAPI. It is really annoying how there
> is just too much variation among network drivers.
Agreed this variation has created a mess and makes it difficult to add
this sort of functionality. But I'm not sure what the "fix" would be.
For instance...
It seems like there are three primary points points where drivers
interact with stack to stop or wakeup queue:
1) On start-xmit, availability is checked and queue is stopped if no
space available queue is stopped. Some drivers report an error if
there was no availability and queue had not been stopped as expected.
2) At the end of start-xmit availability is checked and queue is
stopped if necessary.
3) At end of completion processing, availability is checked and queue
is waked up if necessary.
Each of these are several lines of code in each driver, and seem to be
begging for a unified abstraction. For instance in tg3 instead of:
if (unlikely(netif_tx_queue_stopped(txq) &&
(tg3_tx_avail(tnapi) > TG3_TX_WAKEUP_THRESH(tnapi)))) {
__netif_tx_lock(txq, smp_processor_id());
if (netif_tx_queue_stopped(txq) &&
(tg3_tx_avail(tnapi) > TG3_TX_WAKEUP_THRESH(tnapi)))
netif_tx_wake_queue(txq);
__netif_tx_unlock(txq);
}
we probably would want something like:
netif_tx_queue_complete_check(txq, pkts, bytes, tg3_tx_avail(tnapi) >
TG3_TX_WAKEUP_THRESH(tnapi))
which would do all the work to check for available space, wakeup the
queue, statistic, bql, etc...
I actually did consider abstracting these out as part of bql, but even
though drivers implemented these in similar ways, variations in each
driver might make introducing a unified abstraction nontrivial (like
where drivers need to insert memory barriers and variations in
locking).
^ permalink raw reply
* Re: [PATCH v2] net: add Documentation/networking/scaling.txt
From: Rick Jones @ 2011-08-09 18:45 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: rdunlap, linux-doc, davem, netdev, therbert
In-Reply-To: <1312899648.5889.14.camel@gopher.nyc.corp.google.com>
On 08/09/2011 07:20 AM, Willem de Bruijn wrote:
> Describes RSS, RPS, RFS, accelerated RFS, and XPS.
>
> This version incorporates comments by Randy Dunlap and Rick Jones.
> Besides text cleanup, it adds an explicit "Suggested Configuration"
> heading to each section.
>
> Signed-off-by: Willem de Bruijn<willemb@google.com>
Any other worries I have a probably a matter of opinion or ignorance on
my part.
Acked-By: Rick Jones <rick.jones2@hp.com>
rick jones
^ permalink raw reply
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Robin Holt @ 2011-08-09 18:45 UTC (permalink / raw)
To: Scott Wood
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, U Bhaskar-B22300,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, Marc Kleine-Budde,
PPC list, Wolfgang Grandegger
In-Reply-To: <4E4179CB.6030101-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
On Tue, Aug 09, 2011 at 01:17:47PM -0500, Scott Wood wrote:
> On 08/09/2011 09:43 AM, Robin Holt wrote:
> > In working with the socketcan developers, we have come to the conclusion
> > the fsl-flexcan device tree bindings need to be cleaned up.
> > The driver does not depend upon any properties other than the required properties
> > so we are removing the file.
>
> That is not the criterion for whether something should be expresed in
> the device tree. It's a description of the hardware, not a Linux driver
> configuration file. If there are integration parameters that can not be
> inferred from "this is FSL flexcan v1.0", they should be expressed in
> the node.
There are no properties other than the required properties. The others
were wrongly introduced and are not needed by the driver. When we
removed the other properties and the wrong documentation of the mscan
oscillator source in the fsl-flexcan.txt file, we were left with an
Example: section and a one-line statement "The only properties supported
are the required properties." That seemed like the fsl-flexcan.txt
file was then pointless.
> Removing the binding altogether seems extreme as well -- we should have
> bindings for all devices, even if there are no special properties.
Ok. I can do that too. Who is the definitive source for that answer?
I assume we are talking about the fsl-flexcan.txt file when we say
binding. Is that correct?
> > Additionally, the p1010*dts files are not
> > following the standard for node naming in that they have a trailing -v1.0.
>
> What "standard for node naming"? There's nothing wrong with putting a
For the answer to that, you will need to ask Wolfgang Grandegger. I was
working from his feedback. Looking at the plethora of other node names,
the vast majority do not have any -v#.#, and the ones that do also tend
to have multiple versions. Based upon that, I suspect he is correct,
but I do not know where the documentation is or if it even exists.
> block version number in the compatible string, and it looks like the
> p1010 dts files were following the binding document in this regard. It
> is common practice when the block version is publicly documented but
> there's no register it can be read from at runtime.
Thanks,
Robin
^ permalink raw reply
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Scott Wood @ 2011-08-09 19:13 UTC (permalink / raw)
To: Robin Holt
Cc: Marc Kleine-Budde, Wolfgang Grandegger, U Bhaskar-B22300, netdev,
socketcan-core, PPC list, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <20110809184524.GB4926@sgi.com>
On 08/09/2011 01:45 PM, Robin Holt wrote:
> On Tue, Aug 09, 2011 at 01:17:47PM -0500, Scott Wood wrote:
>> On 08/09/2011 09:43 AM, Robin Holt wrote:
>>> In working with the socketcan developers, we have come to the conclusion
>>> the fsl-flexcan device tree bindings need to be cleaned up.
>>> The driver does not depend upon any properties other than the required properties
>>> so we are removing the file.
>>
>> That is not the criterion for whether something should be expresed in
>> the device tree. It's a description of the hardware, not a Linux driver
>> configuration file. If there are integration parameters that can not be
>> inferred from "this is FSL flexcan v1.0", they should be expressed in
>> the node.
>
> There are no properties other than the required properties. The others
> were wrongly introduced and are not needed by the driver.
Not needed by this driver, or will never be needed by any reasonable
driver (or is not a good description of the hardware)?
The device tree is not an internal Linux implementation detail. It is
shared by other OSes, firmwares, hypervisors, etc. Bindings should be
created with care, and kept stable unless there's a good reason to break
compatibility.
devicetree-discuss@lists.ozlabs.org should be CCed on device tree
discussions.
> When we
> removed the other properties and the wrong documentation of the mscan
> oscillator source in the fsl-flexcan.txt file, we were left with an
> Example: section and a one-line statement "The only properties supported
> are the required properties." That seemed like the fsl-flexcan.txt
> file was then pointless.
There is the compatible string, and you could mention that there is a
single reg resource and a single interrupt.
>> Removing the binding altogether seems extreme as well -- we should have
>> bindings for all devices, even if there are no special properties.
>
> Ok. I can do that too. Who is the definitive source for that answer?
For policy questions on device tree bindings? Grant Likely is the
maintainer for device tree stuff.
A lot of the simpler bindings have been left undocumented so far, IMHO
it should be a goal to document them all. There are some existing ones
that are documented despite not having special properties, e.g.
Documentation/devicetree/bindings/serio/altera_ps2.txt,
Documentation/devicetree/bindings/arm/sirf.txt,
Documentation/devicetree/bindings/powerpc/nintendo/wii.txt, etc.
> I assume we are talking about the fsl-flexcan.txt file when we say
> binding. Is that correct?
Yes, although devicetree.org is another possibility.
>>> Additionally, the p1010*dts files are not
>>> following the standard for node naming in that they have a trailing -v1.0.
>>
>> What "standard for node naming"? There's nothing wrong with putting a
>
> For the answer to that, you will need to ask Wolfgang Grandegger. I was
> working from his feedback. Looking at the plethora of other node names,
> the vast majority do not have any -v#.#, and the ones that do also tend
> to have multiple versions. Based upon that, I suspect he is correct,
> but I do not know where the documentation is or if it even exists.
There's a lot of crap in old bindings, plus it's not appropriate for all
circumstances (specifying bindings should be done a little more
carefully than "what do most other bindings do?"). It's something we've
been doing lately for blocks that have a version number, but it's not
dynamically readable.
Looking in the FlexCAN chapter of the p1010 manual, I don't see any
reference to a block version, and I do see references to "previous
FlexCAN versions". So I suggest "fsl,p1010-flexcan".
-Scott
^ permalink raw reply
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Wolfgang Grandegger @ 2011-08-09 19:32 UTC (permalink / raw)
To: Scott Wood
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
Devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
U Bhaskar-B22300, socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
Marc Kleine-Budde, PPC list
In-Reply-To: <4E4179CB.6030101-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
On 08/09/2011 08:17 PM, Scott Wood wrote:
> On 08/09/2011 09:43 AM, Robin Holt wrote:
>> In working with the socketcan developers, we have come to the conclusion
>> the fsl-flexcan device tree bindings need to be cleaned up.
>> The driver does not depend upon any properties other than the required properties
>> so we are removing the file.
>
> That is not the criterion for whether something should be expresed in
> the device tree. It's a description of the hardware, not a Linux driver
> configuration file. If there are integration parameters that can not be
> inferred from "this is FSL flexcan v1.0", they should be expressed in
> the node.
>
> Removing the binding altogether seems extreme as well -- we should have
> bindings for all devices, even if there are no special properties.
Yes, of course. The commit message misleading. We do not intend to
remove the binding but just a few unused and confusing properties.
Concerning the compatible string, Freescale introduced for the Flexcan
on the P1010 "fsl,flexcan-v1.0". That's not the usual convention also
because the v1.0 if for the PowerPC cores only, I assume, but we have
ARM cores as well. If we need to distinguish I think we should use:
"fsl,p1010-flexcan", "fsl,flexcan"
Do you agree?
>> Additionally, the p1010*dts files are not
>> following the standard for node naming in that they have a trailing -v1.0.
>
> What "standard for node naming"? There's nothing wrong with putting a
> block version number in the compatible string, and it looks like the
> p1010 dts files were following the binding document in this regard. It
> is common practice when the block version is publicly documented but
> there's no register it can be read from at runtime.
See above.
Furthermore I must admit, that the bindings shown up mainline Linux have
never been presented on any mailing list.
Wolfgang.
^ permalink raw reply
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Wolfgang Grandegger @ 2011-08-09 19:49 UTC (permalink / raw)
To: Scott Wood
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
U Bhaskar-B22300, socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
Marc Kleine-Budde, PPC list
In-Reply-To: <4E4186BD.5000602-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
On 08/09/2011 09:13 PM, Scott Wood wrote:
> On 08/09/2011 01:45 PM, Robin Holt wrote:
>> On Tue, Aug 09, 2011 at 01:17:47PM -0500, Scott Wood wrote:
>>> On 08/09/2011 09:43 AM, Robin Holt wrote:
>>>> In working with the socketcan developers, we have come to the conclusion
>>>> the fsl-flexcan device tree bindings need to be cleaned up.
>>>> The driver does not depend upon any properties other than the required properties
>>>> so we are removing the file.
>>>
>>> That is not the criterion for whether something should be expresed in
>>> the device tree. It's a description of the hardware, not a Linux driver
>>> configuration file. If there are integration parameters that can not be
>>> inferred from "this is FSL flexcan v1.0", they should be expressed in
>>> the node.
>>
>> There are no properties other than the required properties. The others
>> were wrongly introduced and are not needed by the driver.
>
> Not needed by this driver, or will never be needed by any reasonable
> driver (or is not a good description of the hardware)?
>
> The device tree is not an internal Linux implementation detail. It is
> shared by other OSes, firmwares, hypervisors, etc. Bindings should be
> created with care, and kept stable unless there's a good reason to break
> compatibility.
>
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org should be CCed on device tree
> discussions.
Yes. The doc for the bindings we speak about
http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
sneaked into the kernel without been presented on any mailing list and
without the corresponding driver patch.
>> When we
>> removed the other properties and the wrong documentation of the mscan
>> oscillator source in the fsl-flexcan.txt file, we were left with an
>> Example: section and a one-line statement "The only properties supported
>> are the required properties." That seemed like the fsl-flexcan.txt
>> file was then pointless.
>
> There is the compatible string, and you could mention that there is a
> single reg resource and a single interrupt.
>
>>> Removing the binding altogether seems extreme as well -- we should have
>>> bindings for all devices, even if there are no special properties.
>>
>> Ok. I can do that too. Who is the definitive source for that answer?
>
> For policy questions on device tree bindings? Grant Likely is the
> maintainer for device tree stuff.
>
> A lot of the simpler bindings have been left undocumented so far, IMHO
> it should be a goal to document them all. There are some existing ones
> that are documented despite not having special properties, e.g.
> Documentation/devicetree/bindings/serio/altera_ps2.txt,
> Documentation/devicetree/bindings/arm/sirf.txt,
> Documentation/devicetree/bindings/powerpc/nintendo/wii.txt, etc.
>
>> I assume we are talking about the fsl-flexcan.txt file when we say
>> binding. Is that correct?
>
> Yes, although devicetree.org is another possibility.
>
>>>> Additionally, the p1010*dts files are not
>>>> following the standard for node naming in that they have a trailing -v1.0.
>>>
>>> What "standard for node naming"? There's nothing wrong with putting a
>>
>> For the answer to that, you will need to ask Wolfgang Grandegger. I was
>> working from his feedback. Looking at the plethora of other node names,
>> the vast majority do not have any -v#.#, and the ones that do also tend
>> to have multiple versions. Based upon that, I suspect he is correct,
>> but I do not know where the documentation is or if it even exists.
>
> There's a lot of crap in old bindings, plus it's not appropriate for all
> circumstances (specifying bindings should be done a little more
> carefully than "what do most other bindings do?"). It's something we've
> been doing lately for blocks that have a version number, but it's not
> dynamically readable.
>
> Looking in the FlexCAN chapter of the p1010 manual, I don't see any
> reference to a block version, and I do see references to "previous
> FlexCAN versions". So I suggest "fsl,p1010-flexcan".
OK, just
"fsl,p1010-flexcan"
or
"fsl,p1010-flexcan", "fsl,flexcan"
Note that the Flexcan is used on Freescale ARM cores as well (and device
tree for ARM will show up soon).
Wolfgang.
^ permalink raw reply
* Re: [PATCH] gianfar: reduce stack usage in gianfar_ethtool.c
From: Joe Perches @ 2011-08-09 19:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, stufever, netdev, davem, Wang Shaoyan
In-Reply-To: <1312914257.2547.10.camel@edumazet-laptop>
On Tue, 2011-08-09 at 20:24 +0200, Eric Dumazet wrote:
> Le mardi 09 août 2011 à 11:12 -0700, Joe Perches a écrit :
> > On Tue, 2011-08-09 at 19:59 +0200, Eric Dumazet wrote:
> > > Le mardi 09 août 2011 à 10:52 -0700, Joe Perches a écrit :
> > > > On Tue, 2011-08-09 at 18:53 +0200, Eric Dumazet wrote:
> > > > > Le mercredi 10 août 2011 à 00:39 +0800, stufever@gmail.com a écrit :
> > > > []
> > > > > > + if (!local_rqfpr || !local_rqfcr) {
> > > > > > + pr_err("Out of memory\n");
> > > > > Please remove this pr_err(), kmalloc() will complain already.
I think this is fine and should be kept until
some general agreement is made that OOM messages
should be removed generically.
If these are really superfluous, which I doubt a
little because these are emitted at different
KERN_<LEVEL>, (the generic one emits at KERN_WARNING),
there are _thousands_ of these OOM errors in drivers/
alone that could be removed.
$ grep -rP --include=*.[ch] \
"(printk|\b[a-z]+_\w+)\s*\(.*\".*(alloc|mem)" drivers | \
wc -l
5147
call it 50% false positive, it's still a lot.
I think one more won't hurt for awhile.
^ permalink raw reply
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Scott Wood @ 2011-08-09 19:58 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Robin Holt, Marc Kleine-Budde, U Bhaskar-B22300, netdev,
socketcan-core, PPC list, devicetree-discuss@lists.ozlabs.org
In-Reply-To: <4E418F2D.4060504@grandegger.com>
On 08/09/2011 02:49 PM, Wolfgang Grandegger wrote:
> Yes. The doc for the bindings we speak about
>
> http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
>
> sneaked into the kernel without been presented on any mailing list and
> without the corresponding driver patch.
It was posted on linuxppc-dev:
http://patchwork.ozlabs.org/patch/91980/
Though I agree it should have been posted more widely.
> OK, just
>
> "fsl,p1010-flexcan"
>
> or
>
> "fsl,p1010-flexcan", "fsl,flexcan"
I'm ok with the latter, if there's enough in common that it's
conceivable that a driver wouldn't care. The more specific compatible
will be there if the driver wants to make use of it later.
-Scot
^ permalink raw reply
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Scott Wood @ 2011-08-09 20:11 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: netdev, Devicetree-discuss@lists.ozlabs.org, U Bhaskar-B22300,
socketcan-core, Robin Holt, PPC list
In-Reply-To: <4E418B39.6040408@grandegger.com>
On 08/09/2011 02:32 PM, Wolfgang Grandegger wrote:
> On 08/09/2011 08:17 PM, Scott Wood wrote:
>> On 08/09/2011 09:43 AM, Robin Holt wrote:
>>> In working with the socketcan developers, we have come to the conclusion
>>> the fsl-flexcan device tree bindings need to be cleaned up.
>>> The driver does not depend upon any properties other than the required properties
>>> so we are removing the file.
>>
>> That is not the criterion for whether something should be expresed in
>> the device tree. It's a description of the hardware, not a Linux driver
>> configuration file. If there are integration parameters that can not be
>> inferred from "this is FSL flexcan v1.0", they should be expressed in
>> the node.
>>
>> Removing the binding altogether seems extreme as well -- we should have
>> bindings for all devices, even if there are no special properties.
>
> Yes, of course. The commit message misleading. We do not intend to
> remove the binding but just a few unused and confusing properties.
Is it a matter of the current driver not caring, or the properties just
not making sense for any reasonable driver (ambiguous, inferrable from
the flexcan version, software configuration, etc)?
-Scott
^ permalink raw reply
* Re: [PATCHv3 4/9] macb: convert printk to netdev_ and friends
From: Joe Perches @ 2011-08-09 20:20 UTC (permalink / raw)
To: Jamie Iles; +Cc: netdev, linux-arm-kernel, plagnioj
In-Reply-To: <1312881411-2376-5-git-send-email-jamie@jamieiles.com>
On Tue, 2011-08-09 at 10:16 +0100, Jamie Iles wrote:
> macb is already using the dev_dbg() and friends helpers so use netdev_()
> along with a pr_fmt() definition to make the printing a little cleaner.
[]
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
[]
> @@ -8,6 +8,7 @@
[]
> +#define pr_fmt(fmt) "macb: " fmt
Hi Jamie.
Just trivia.
Please use #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
instead. There would be no output difference.
That'll make it easier to remove later when all of
of these #defines pr_fmt that use just KBUILD_MODNAME
can be removed.
^ permalink raw reply
* Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.
From: Robin Holt @ 2011-08-09 20:59 UTC (permalink / raw)
To: Scott Wood
Cc: Wolfgang Grandegger, Robin Holt, Marc Kleine-Budde,
U Bhaskar-B22300, netdev, socketcan-core, PPC list,
devicetree-discuss@lists.ozlabs.org
In-Reply-To: <4E419180.3090507@freescale.com>
I guess my poor wording may have gotten me in trouble. I am getting
ready to repost this patch, but I want to ensure I am getting it as
right as possible.
I think I should reword the commit message to indicate we are removing
the Documentation/.../fsl-flexcan.txt file which has essentially become
empty and change the p1010si.dtsi file's can nodes to "fsl,p1010-flexcan",
"fsl,flexcan". Is that correct?
Thanks,
Robin
On Tue, Aug 09, 2011 at 02:58:56PM -0500, Scott Wood wrote:
> On 08/09/2011 02:49 PM, Wolfgang Grandegger wrote:
> > Yes. The doc for the bindings we speak about
> >
> > http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >
> > sneaked into the kernel without been presented on any mailing list and
> > without the corresponding driver patch.
>
> It was posted on linuxppc-dev:
> http://patchwork.ozlabs.org/patch/91980/
>
> Though I agree it should have been posted more widely.
>
> > OK, just
> >
> > "fsl,p1010-flexcan"
> >
> > or
> >
> > "fsl,p1010-flexcan", "fsl,flexcan"
>
> I'm ok with the latter, if there's enough in common that it's
> conceivable that a driver wouldn't care. The more specific compatible
> will be there if the driver wants to make use of it later.
>
> -Scot
^ permalink raw reply
* Re: [PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.
From: Shreyas Bhatewara @ 2011-08-09 21:32 UTC (permalink / raw)
To: Jesse Gross
Cc: David Miller, netdev@vger.kernel.org, Scott Goldman,
VMware PV-Drivers
In-Reply-To: <1312794947-3163-1-git-send-email-jesse@nicira.com>
On Mon, 8 Aug 2011, Jesse Gross wrote:
> @@ -1929,14 +1929,17 @@ static void
> vmxnet3_vlan_rx_add_vid(struct net_device *netdev, u16 vid)
> {
> struct vmxnet3_adapter *adapter = netdev_priv(netdev);
> - u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
> - unsigned long flags;
>
> - VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
> - spin_lock_irqsave(&adapter->cmd_lock, flags);
> - VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
> - VMXNET3_CMD_UPDATE_VLAN_FILTERS);
> - spin_unlock_irqrestore(&adapter->cmd_lock, flags);
> + if (!(netdev->flags & IFF_PROMISC)) {
> + u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
> + unsigned long flags;
> +
> + VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
> + spin_lock_irqsave(&adapter->cmd_lock, flags);
> + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
> + VMXNET3_CMD_UPDATE_VLAN_FILTERS);
> + spin_unlock_irqrestore(&adapter->cmd_lock, flags);
> + }
>
If this is done, the driver will ignore all vlan tag registrations (and
deletions) while the interface is in promiscuous mode. Better solution
would be to send UPDATE_VLAN_FILTERS command alone inside the promiscuous
condition. vfTable can be set/unset unconditionally as before. By doing
this, when the interface comes out of promiscuous mode, the restored vlan
state will have all the added/removed vlan tags into effect.
^ permalink raw reply
* Re: [Patch] scm: Capture the full credentials of the scm sender
From: James Morris @ 2011-08-09 23:20 UTC (permalink / raw)
To: Tim Chen
Cc: Eric Dumazet, Eric W. Biederman, David S. Miller, Al Viro, ak,
linux-kernel, netdev
In-Reply-To: <1312908512.2576.97.camel@schen9-DESK>
On Tue, 9 Aug 2011, Tim Chen wrote:
> This patch corrects an erroneous update of credential's gid with uid
> introduced in commit 257b5358b32f17 since 2.6.36.
>
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: James Morris <jmorris@namei.org>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH net] vmxnet3: Don't enable vlan filters in promiscuous mode.
From: Jesse Gross @ 2011-08-10 0:16 UTC (permalink / raw)
To: Shreyas Bhatewara
Cc: David Miller, netdev@vger.kernel.org, Scott Goldman,
VMware PV-Drivers
In-Reply-To: <alpine.LRH.2.00.1108091426010.27839@sbhatewara-dev1.eng.vmware.com>
On Wed, Aug 10, 2011 at 5:32 AM, Shreyas Bhatewara
<sbhatewara@vmware.com> wrote:
>
>
> On Mon, 8 Aug 2011, Jesse Gross wrote:
>
>> @@ -1929,14 +1929,17 @@ static void
>> vmxnet3_vlan_rx_add_vid(struct net_device *netdev, u16 vid)
>> {
>> struct vmxnet3_adapter *adapter = netdev_priv(netdev);
>> - u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
>> - unsigned long flags;
>>
>> - VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
>> - spin_lock_irqsave(&adapter->cmd_lock, flags);
>> - VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
>> - VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>> - spin_unlock_irqrestore(&adapter->cmd_lock, flags);
>> + if (!(netdev->flags & IFF_PROMISC)) {
>> + u32 *vfTable = adapter->shared->devRead.rxFilterConf.vfTable;
>> + unsigned long flags;
>> +
>> + VMXNET3_SET_VFTABLE_ENTRY(vfTable, vid);
>> + spin_lock_irqsave(&adapter->cmd_lock, flags);
>> + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
>> + VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>> + spin_unlock_irqrestore(&adapter->cmd_lock, flags);
>> + }
>>
>
> If this is done, the driver will ignore all vlan tag registrations (and
> deletions) while the interface is in promiscuous mode. Better solution
> would be to send UPDATE_VLAN_FILTERS command alone inside the promiscuous
> condition. vfTable can be set/unset unconditionally as before. By doing
> this, when the interface comes out of promiscuous mode, the restored vlan
> state will have all the added/removed vlan tags into effect.
Adds and removes are not ignored when in promiscuous mode because the
active_vlans bitfield is still being updated (this is in the context
that you clipped out). When we come out of promiscuous mode,
vmxnet3_restore_vlan() is called which will update the hardware with
the vlans that have been registered. This is much safer than directly
updating vfTable because it will get overwritten if vmxnet3_set_mc()
is called again and is potentially very error prone to have it not
reflect what we actually want programmed in the hardware.
^ permalink raw reply
* Re: [PATCH 1/6] Security: define security_sk_getsecid.
From: Rongqing Li @ 2011-08-10 0:43 UTC (permalink / raw)
To: Casey Schaufler; +Cc: netdev, selinux, linux-security-module, sds
In-Reply-To: <4E415CB3.8020202@schaufler-ca.com>
On 08/10/2011 12:13 AM, Casey Schaufler wrote:
> On 8/9/2011 12:28 AM, rongqing.li@windriver.com wrote:
>> From: Roy.Li<rongqing.li@windriver.com>
>>
>> Define security_sk_getsecid to get the security id of a sock.
>
> Why are you requesting the secid when you're just going to
> use it to get the secctx? Why not ask for that directly?
> Is there ever a case where you only want the secid?
>
Hi:
As I know, we have not method to get secctx directly.
On the most of time, we get secctx like this.
The below comes from kernel/auditsc.c
void audit_log_task_context(struct audit_buffer *ab)
{
char *ctx = NULL;
unsigned len;
int error;
u32 sid;
security_task_getsecid(current, &sid);
if (!sid)
return;
error = security_secid_to_secctx(sid, &ctx, &len);
if (error) {
if (error != -EINVAL)
goto error_path;
return;
}
audit_log_format(ab, " subj=%s", ctx);
security_release_secctx(ctx, len);
return;
error_path:
audit_panic("error in audit_log_task_context");
return;
}
-Roy
>>
>> Signed-off-by: Roy.Li<rongqing.li@windriver.com>
>> ---
>> include/linux/security.h | 6 ++++++
>> security/security.c | 6 ++++++
>> 2 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index ebd2a53..739ac39 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -2560,6 +2560,7 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
>> void security_sk_free(struct sock *sk);
>> void security_sk_clone(const struct sock *sk, struct sock *newsk);
>> void security_sk_classify_flow(struct sock *sk, struct flowi *fl);
>> +void security_sk_getsecid(struct sock *sk, u32 *secid);
>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl);
>> void security_sock_graft(struct sock*sk, struct socket *parent);
>> int security_inet_conn_request(struct sock *sk,
>> @@ -2701,6 +2702,11 @@ static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>> {
>> }
>>
>> +static inline void security_sk_getsecid(struct sock *sk, u32 *secid)
>> +{
>> + *secid = 0;
>> +}
>> +
>> static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>> {
>> }
>> diff --git a/security/security.c b/security/security.c
>> index 0e4fccf..b0e0825 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -1104,6 +1104,12 @@ void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>> }
>> EXPORT_SYMBOL(security_sk_classify_flow);
>>
>> +void security_sk_getsecid(struct sock *sk, u32 *secid)
>> +{
>> + security_ops->sk_getsecid(sk, secid);
>> +}
>> +EXPORT_SYMBOL(security_sk_getsecid);
>> +
>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>> {
>> security_ops->req_classify_flow(req, fl);
>
>
--
Best Reagrds,
Roy | RongQing Li
^ permalink raw reply
* BUG: Bisected Gianfar in bridge not forwarding packets (was: 3.0-rc1 Bridge not forwarding unicast packages)
From: Michael Guntsche @ 2011-08-10 0:50 UTC (permalink / raw)
To: shemminger, jpirko, sebastian.belden; +Cc: netdev, linux-kernel
Good evening/morning,
> I just upgraded my router/bridge combo to 3.1-rc1 from 3.0 for
> testing. On a first look everything seemed to work fine, but when I
> tried to connect via openvpn to my internal network (tap0 being bridged
> with the internal network) I noticed that I was not able to access the
> server on my internal network. I could access the bridge (which is
> acting as the openvpn server as well) just fine though.
> To debug this I ran tcpdump on the openvpn client and started a ping to the
> internal network. I could see the ARP requests being answered.....
<snip>
After much digging and testing I found the commit that causes the
problem for me here.
87c288c6e gianfar: do vlan cleanup
Before this commit my bridge works fine. I simplified the test case
and just tried to access the server from a local wireless device with
gets also bridged via
a gianfar nic to the wired lan. Before commit 87c288c6e I can ping the
device from the internal network afterwards it stops.
For testing purposes I reverted this commit (plus some others to make
the code compile) and it started working again.
The hardware is a routerboard with three onboard nics two of em gianfar.
The gianfar nics do not support any kind of offloading,
Offload parameters for lan_wire:
rx-checksumming: off
tx-checksumming: off
scatter-gather: off
tcp-segmentation-offload: off
udp-fragmentation-offload: off
generic-segmentation-offload: off
generic-receive-offload: on
large-receive-offload: off
rx-vlan-offload: off
tx-vlan-offload: off
ntuple-filters: off
receive-hashing: off
The Bridge device on the other hand....
Offload parameters for lan:
rx-checksumming: on
tx-checksumming: on
scatter-gather: off
tcp-segmentation-offload: off
udp-fragmentation-offload: off
generic-segmentation-offload: off
generic-receive-offload: on
large-receive-offload: off
rx-vlan-offload: off
tx-vlan-offload: on
ntuple-filters: off
receive-hashing: off
Is it possible that the new code is tripped up by the bridge offload
parameters somehow and thinks that the gianfar nic supports it as
well?
As I wrote in my initial mail, connecting to the Bridge itself works,
I can also see ARP requests and Broadcasts, the rest just seems to
disappear.
As for the commits I reverted:
a4aeb2662 vlan: kill __vlan_hwaccel_rx and vlan_hwaccel_rx
6dacaddd4 vlan: kill vlan_hwaccel_receive_skb
7890a5b9c vlan: kill ndo_vlan_rx_register
b852b7208 gianfar: fix bug caused by 87c288c6e9aa31720b72e2bc2d665e24e1653c3e
87c288c6e gianfar: do vlan cleanup
I really think that there is only some small change in the gianfar
code needed, but I could not figure it out myself.
Please tell me if you need any more information.
Kind regards,
Michael Guntsche
^ permalink raw reply
* Re: [PATCH 1/6] Security: define security_sk_getsecid.
From: Casey Schaufler @ 2011-08-10 0:57 UTC (permalink / raw)
To: Rongqing Li; +Cc: netdev, selinux, linux-security-module, sds
In-Reply-To: <4E41D421.1000302@windriver.com>
On 8/9/2011 5:43 PM, Rongqing Li wrote:
> On 08/10/2011 12:13 AM, Casey Schaufler wrote:
>> On 8/9/2011 12:28 AM, rongqing.li@windriver.com wrote:
>>> From: Roy.Li<rongqing.li@windriver.com>
>>>
>>> Define security_sk_getsecid to get the security id of a sock.
>>
>> Why are you requesting the secid when you're just going to
>> use it to get the secctx? Why not ask for that directly?
>> Is there ever a case where you only want the secid?
>>
> Hi:
>
> As I know, we have not method to get secctx directly.
You are defining the method! Ask for what you want!
The whole notion of secids is a holdover from the bad old
days when SELinux was a user space based enforcement mechanism.
The audit system was implemented when SELinux was the lone LSM
and unfortunately and unnecessarily propagated the use of secids.
If an object has a secid it must also have a secctx. The
interfaces that use secids could just as well use the secctx.
It is wasteful to create a new interface that fetches a secid
just to turn around and ask for the secctx in all cases.
> On the most of time, we get secctx like this.
>
> The below comes from kernel/auditsc.c
>
> void audit_log_task_context(struct audit_buffer *ab)
> {
> char *ctx = NULL;
> unsigned len;
> int error;
> u32 sid;
>
> security_task_getsecid(current, &sid);
> if (!sid)
> return;
>
> error = security_secid_to_secctx(sid, &ctx, &len);
> if (error) {
> if (error != -EINVAL)
> goto error_path;
> return;
> }
>
> audit_log_format(ab, " subj=%s", ctx);
> security_release_secctx(ctx, len);
> return;
>
> error_path:
> audit_panic("error in audit_log_task_context");
> return;
> }
>
>
> -Roy
>
>
>>>
>>> Signed-off-by: Roy.Li<rongqing.li@windriver.com>
>>> ---
>>> include/linux/security.h | 6 ++++++
>>> security/security.c | 6 ++++++
>>> 2 files changed, 12 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>> index ebd2a53..739ac39 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -2560,6 +2560,7 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
>>> void security_sk_free(struct sock *sk);
>>> void security_sk_clone(const struct sock *sk, struct sock *newsk);
>>> void security_sk_classify_flow(struct sock *sk, struct flowi *fl);
>>> +void security_sk_getsecid(struct sock *sk, u32 *secid);
>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl);
>>> void security_sock_graft(struct sock*sk, struct socket *parent);
>>> int security_inet_conn_request(struct sock *sk,
>>> @@ -2701,6 +2702,11 @@ static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>> {
>>> }
>>>
>>> +static inline void security_sk_getsecid(struct sock *sk, u32 *secid)
>>> +{
>>> + *secid = 0;
>>> +}
>>> +
>>> static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>> {
>>> }
>>> diff --git a/security/security.c b/security/security.c
>>> index 0e4fccf..b0e0825 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -1104,6 +1104,12 @@ void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>> }
>>> EXPORT_SYMBOL(security_sk_classify_flow);
>>>
>>> +void security_sk_getsecid(struct sock *sk, u32 *secid)
>>> +{
>>> + security_ops->sk_getsecid(sk, secid);
>>> +}
>>> +EXPORT_SYMBOL(security_sk_getsecid);
>>> +
>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>> {
>>> security_ops->req_classify_flow(req, fl);
>>
>>
>
^ permalink raw reply
* Re: [PATCH] Fix RCU warning in rt_cache_seq_show
From: Paul E. McKenney @ 2011-08-10 1:11 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Mark Rutland, netdev, David S. Miller, Gergely Kalman
In-Reply-To: <1312910336.2371.61.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Tue, Aug 09, 2011 at 07:18:56PM +0200, Eric Dumazet wrote:
> Le mardi 09 août 2011 à 18:02 +0100, Mark Rutland a écrit :
> > Commit f2c31e32 ("net: fix NULL dereferences in check_peer_redir()")
> > added rcu protection to dst neighbour, and updated callsites for
> > dst_{get,set}_neighbour. Unfortunately, it missed rt_cache_seq_show.
> >
> > This produces a warning on v3.1-rc1 (on a preemptible kernel, on an
> > ARM Vexpress A9x4):
> >
> > ===================================================
> > [ INFO: suspicious rcu_dereference_check() usage. ]
> > ---------------------------------------------------
> > include/net/dst.h:91 invoked rcu_dereference_check() without protection!
> >
> > other info that might help us debug this:
> >
> > rcu_scheduler_active = 1, debug_locks = 0
> > 2 locks held by proc01/32159:
This is very strange. It says that there are two locks held by the
task, but refuses to list them. Maybe something stomped on the list
of held locks?
Thanx, Paul
> > stack backtrace:
> > [<80014880>] (unwind_backtrace+0x0/0xf8) from [<802e5c78>] (rt_cache_seq_show+0x18c/0x1c4)
> > [<802e5c78>] (rt_cache_seq_show+0x18c/0x1c4) from [<800e0c5c>] (seq_read+0x324/0x4a4)
> > [<800e0c5c>] (seq_read+0x324/0x4a4) from [<8010786c>] (proc_reg_read+0x70/0x94)
> > [<8010786c>] (proc_reg_read+0x70/0x94) from [<800c0ba8>] (vfs_read+0xb0/0x144)
> > [<800c0ba8>] (vfs_read+0xb0/0x144) from [<800c0ea8>] (sys_read+0x40/0x70)
> > [<800c0ea8>] (sys_read+0x40/0x70) from [<8000e0c0>] (ret_fast_syscall+0x0/0x3c)
> >
> > This patch adds calls to rcu_read_{lock,unlock} in rt_cache_seq_show,
> > protecting the dereferenced variable, and clearing the warning.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Gergely Kalman <synapse@hippy.csoma.elte.hu>
> > ---
> > net/ipv4/route.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index e3dec1c..6699ef7 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -419,6 +419,7 @@ static int rt_cache_seq_show(struct seq_file *seq, void *v)
> > struct neighbour *n;
> > int len;
> >
> > + rcu_read_lock();
> > n = dst_get_neighbour(&r->dst);
> > seq_printf(seq, "%s\t%08X\t%08X\t%8X\t%d\t%u\t%d\t"
> > "%08X\t%d\t%u\t%u\t%02X\t%d\t%1d\t%08X%n",
> > @@ -435,6 +436,7 @@ static int rt_cache_seq_show(struct seq_file *seq, void *v)
> > -1,
> > (n && (n->nud_state & NUD_CONNECTED)) ? 1 : 0,
> > r->rt_spec_dst, &len);
> > + rcu_read_unlock();
> >
> > seq_printf(seq, "%*s\n", 127 - len, "");
> > }
>
>
> Hmm, I though rcu_read_lock_bh() (done by caller of this function) was
> protecting us here.
>
>
> Paul, is it really needed, or is it a lockdep artifact ?
>
>
>
>
>
^ permalink raw reply
* Re: [PATCH 1/6] Security: define security_sk_getsecid.
From: Rongqing Li @ 2011-08-10 1:24 UTC (permalink / raw)
To: Casey Schaufler; +Cc: netdev, selinux, linux-security-module, sds
In-Reply-To: <4E41D78C.7040007@schaufler-ca.com>
On 08/10/2011 08:57 AM, Casey Schaufler wrote:
> On 8/9/2011 5:43 PM, Rongqing Li wrote:
>> On 08/10/2011 12:13 AM, Casey Schaufler wrote:
>>> On 8/9/2011 12:28 AM, rongqing.li@windriver.com wrote:
>>>> From: Roy.Li<rongqing.li@windriver.com>
>>>>
>>>> Define security_sk_getsecid to get the security id of a sock.
>>>
>>> Why are you requesting the secid when you're just going to
>>> use it to get the secctx? Why not ask for that directly?
>>> Is there ever a case where you only want the secid?
>>>
>> Hi:
>>
>> As I know, we have not method to get secctx directly.
>
> You are defining the method! Ask for what you want!
>
> The whole notion of secids is a holdover from the bad old
> days when SELinux was a user space based enforcement mechanism.
> The audit system was implemented when SELinux was the lone LSM
> and unfortunately and unnecessarily propagated the use of secids.
> If an object has a secid it must also have a secctx. The
> interfaces that use secids could just as well use the secctx.
> It is wasteful to create a new interface that fetches a secid
> just to turn around and ask for the secctx in all cases.
>
Do you means I should write a method like below
security_sk_getsecctx(struct sock *sk, char *secctx, int *len)?
But secctx only is used to user. secid is used to source code to
compute and compare the access permission.
And I do not see the same method like
security_task_getsecctx(). but security_task_getsecid() has been
implemented in kernel source code.
-Roy
>> On the most of time, we get secctx like this.
>>
>> The below comes from kernel/auditsc.c
>>
>> void audit_log_task_context(struct audit_buffer *ab)
>> {
>> char *ctx = NULL;
>> unsigned len;
>> int error;
>> u32 sid;
>>
>> security_task_getsecid(current,&sid);
>> if (!sid)
>> return;
>>
>> error = security_secid_to_secctx(sid,&ctx,&len);
>> if (error) {
>> if (error != -EINVAL)
>> goto error_path;
>> return;
>> }
>>
>> audit_log_format(ab, " subj=%s", ctx);
>> security_release_secctx(ctx, len);
>> return;
>>
>> error_path:
>> audit_panic("error in audit_log_task_context");
>> return;
>> }
>>
>>
>> -Roy
>>
>>
>>>>
>>>> Signed-off-by: Roy.Li<rongqing.li@windriver.com>
>>>> ---
>>>> include/linux/security.h | 6 ++++++
>>>> security/security.c | 6 ++++++
>>>> 2 files changed, 12 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>>> index ebd2a53..739ac39 100644
>>>> --- a/include/linux/security.h
>>>> +++ b/include/linux/security.h
>>>> @@ -2560,6 +2560,7 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
>>>> void security_sk_free(struct sock *sk);
>>>> void security_sk_clone(const struct sock *sk, struct sock *newsk);
>>>> void security_sk_classify_flow(struct sock *sk, struct flowi *fl);
>>>> +void security_sk_getsecid(struct sock *sk, u32 *secid);
>>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl);
>>>> void security_sock_graft(struct sock*sk, struct socket *parent);
>>>> int security_inet_conn_request(struct sock *sk,
>>>> @@ -2701,6 +2702,11 @@ static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>>> {
>>>> }
>>>>
>>>> +static inline void security_sk_getsecid(struct sock *sk, u32 *secid)
>>>> +{
>>>> + *secid = 0;
>>>> +}
>>>> +
>>>> static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>>> {
>>>> }
>>>> diff --git a/security/security.c b/security/security.c
>>>> index 0e4fccf..b0e0825 100644
>>>> --- a/security/security.c
>>>> +++ b/security/security.c
>>>> @@ -1104,6 +1104,12 @@ void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>>> }
>>>> EXPORT_SYMBOL(security_sk_classify_flow);
>>>>
>>>> +void security_sk_getsecid(struct sock *sk, u32 *secid)
>>>> +{
>>>> + security_ops->sk_getsecid(sk, secid);
>>>> +}
>>>> +EXPORT_SYMBOL(security_sk_getsecid);
>>>> +
>>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>>> {
>>>> security_ops->req_classify_flow(req, fl);
>>>
>>>
>>
>
>
--
Best Reagrds,
Roy | RongQing Li
^ permalink raw reply
* Re: [PATCH 1/6] Security: define security_sk_getsecid.
From: Casey Schaufler @ 2011-08-10 1:35 UTC (permalink / raw)
To: Rongqing Li; +Cc: netdev, selinux, linux-security-module, sds, Casey Schaufler
In-Reply-To: <4E41DDEB.9040904@windriver.com>
On 8/9/2011 6:24 PM, Rongqing Li wrote:
> On 08/10/2011 08:57 AM, Casey Schaufler wrote:
>> On 8/9/2011 5:43 PM, Rongqing Li wrote:
>>> On 08/10/2011 12:13 AM, Casey Schaufler wrote:
>>>> On 8/9/2011 12:28 AM, rongqing.li@windriver.com wrote:
>>>>> From: Roy.Li<rongqing.li@windriver.com>
>>>>>
>>>>> Define security_sk_getsecid to get the security id of a sock.
>>>>
>>>> Why are you requesting the secid when you're just going to
>>>> use it to get the secctx? Why not ask for that directly?
>>>> Is there ever a case where you only want the secid?
>>>>
>>> Hi:
>>>
>>> As I know, we have not method to get secctx directly.
>>
>> You are defining the method! Ask for what you want!
>>
>> The whole notion of secids is a holdover from the bad old
>> days when SELinux was a user space based enforcement mechanism.
>> The audit system was implemented when SELinux was the lone LSM
>> and unfortunately and unnecessarily propagated the use of secids.
>> If an object has a secid it must also have a secctx. The
>> interfaces that use secids could just as well use the secctx.
>> It is wasteful to create a new interface that fetches a secid
>> just to turn around and ask for the secctx in all cases.
>>
>
> Do you means I should write a method like below
> security_sk_getsecctx(struct sock *sk, char *secctx, int *len)?
Yes. That is exactly what you should do.
>
> But secctx only is used to user.
But all you're doing is printing out the secctx. The only
thing you are doing with the secid is converting it to a
secctx.
> secid is used to source code to
> compute and compare the access permission.
That will depend on the LSM involved. You are making a change to
the LSM, not just SELinux.
>
> And I do not see the same method like
> security_task_getsecctx(). but security_task_getsecid() has been
> implemented in kernel source code.
Have a look at how those interfaces are used.
>
> -Roy
>
>
>>> On the most of time, we get secctx like this.
>>>
>>> The below comes from kernel/auditsc.c
>>>
>>> void audit_log_task_context(struct audit_buffer *ab)
>>> {
>>> char *ctx = NULL;
>>> unsigned len;
>>> int error;
>>> u32 sid;
>>>
>>> security_task_getsecid(current,&sid);
>>> if (!sid)
>>> return;
>>>
>>> error = security_secid_to_secctx(sid,&ctx,&len);
>>> if (error) {
>>> if (error != -EINVAL)
>>> goto error_path;
>>> return;
>>> }
>>>
>>> audit_log_format(ab, " subj=%s", ctx);
>>> security_release_secctx(ctx, len);
>>> return;
>>>
>>> error_path:
>>> audit_panic("error in audit_log_task_context");
>>> return;
>>> }
>>>
>>>
>>> -Roy
>>>
>>>
>>>>>
>>>>> Signed-off-by: Roy.Li<rongqing.li@windriver.com>
>>>>> ---
>>>>> include/linux/security.h | 6 ++++++
>>>>> security/security.c | 6 ++++++
>>>>> 2 files changed, 12 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>>>> index ebd2a53..739ac39 100644
>>>>> --- a/include/linux/security.h
>>>>> +++ b/include/linux/security.h
>>>>> @@ -2560,6 +2560,7 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
>>>>> void security_sk_free(struct sock *sk);
>>>>> void security_sk_clone(const struct sock *sk, struct sock *newsk);
>>>>> void security_sk_classify_flow(struct sock *sk, struct flowi *fl);
>>>>> +void security_sk_getsecid(struct sock *sk, u32 *secid);
>>>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl);
>>>>> void security_sock_graft(struct sock*sk, struct socket *parent);
>>>>> int security_inet_conn_request(struct sock *sk,
>>>>> @@ -2701,6 +2702,11 @@ static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>>>> {
>>>>> }
>>>>>
>>>>> +static inline void security_sk_getsecid(struct sock *sk, u32 *secid)
>>>>> +{
>>>>> + *secid = 0;
>>>>> +}
>>>>> +
>>>>> static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>>>> {
>>>>> }
>>>>> diff --git a/security/security.c b/security/security.c
>>>>> index 0e4fccf..b0e0825 100644
>>>>> --- a/security/security.c
>>>>> +++ b/security/security.c
>>>>> @@ -1104,6 +1104,12 @@ void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>>>> }
>>>>> EXPORT_SYMBOL(security_sk_classify_flow);
>>>>>
>>>>> +void security_sk_getsecid(struct sock *sk, u32 *secid)
>>>>> +{
>>>>> + security_ops->sk_getsecid(sk, secid);
>>>>> +}
>>>>> +EXPORT_SYMBOL(security_sk_getsecid);
>>>>> +
>>>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>>>> {
>>>>> security_ops->req_classify_flow(req, fl);
>>>>
>>>>
>>>
>>
>>
>
^ permalink raw reply
* Re: [PATCH 1/6] Security: define security_sk_getsecid.
From: Rongqing Li @ 2011-08-10 1:44 UTC (permalink / raw)
To: Casey Schaufler; +Cc: netdev, selinux, linux-security-module, sds
In-Reply-To: <4E41E07C.4080908@schaufler-ca.com>
On 08/10/2011 09:35 AM, Casey Schaufler wrote:
> On 8/9/2011 6:24 PM, Rongqing Li wrote:
>> On 08/10/2011 08:57 AM, Casey Schaufler wrote:
>>> On 8/9/2011 5:43 PM, Rongqing Li wrote:
>>>> On 08/10/2011 12:13 AM, Casey Schaufler wrote:
>>>>> On 8/9/2011 12:28 AM, rongqing.li@windriver.com wrote:
>>>>>> From: Roy.Li<rongqing.li@windriver.com>
>>>>>>
>>>>>> Define security_sk_getsecid to get the security id of a sock.
>>>>>
>>>>> Why are you requesting the secid when you're just going to
>>>>> use it to get the secctx? Why not ask for that directly?
>>>>> Is there ever a case where you only want the secid?
>>>>>
>>>> Hi:
>>>>
>>>> As I know, we have not method to get secctx directly.
>>>
>>> You are defining the method! Ask for what you want!
>>>
>>> The whole notion of secids is a holdover from the bad old
>>> days when SELinux was a user space based enforcement mechanism.
>>> The audit system was implemented when SELinux was the lone LSM
>>> and unfortunately and unnecessarily propagated the use of secids.
>>> If an object has a secid it must also have a secctx. The
>>> interfaces that use secids could just as well use the secctx.
>>> It is wasteful to create a new interface that fetches a secid
>>> just to turn around and ask for the secctx in all cases.
>>>
>>
>> Do you means I should write a method like below
>> security_sk_getsecctx(struct sock *sk, char *secctx, int *len)?
>
> Yes. That is exactly what you should do.
>
>>
>> But secctx only is used to user.
>
> But all you're doing is printing out the secctx. The only
> thing you are doing with the secid is converting it to a
> secctx.
>
>> secid is used to source code to
>> compute and compare the access permission.
>
> That will depend on the LSM involved. You are making a change to
> the LSM, not just SELinux.
>
>>
>> And I do not see the same method like
>> security_task_getsecctx(). but security_task_getsecid() has been
>> implemented in kernel source code.
>
> Have a look at how those interfaces are used.
>
>
Thank you very much.
I will study these interfaces, and hope get your comments when
I send new patches.
Thanks.
>>
>> -Roy
>>
>>
>>>> On the most of time, we get secctx like this.
>>>>
>>>> The below comes from kernel/auditsc.c
>>>>
>>>> void audit_log_task_context(struct audit_buffer *ab)
>>>> {
>>>> char *ctx = NULL;
>>>> unsigned len;
>>>> int error;
>>>> u32 sid;
>>>>
>>>> security_task_getsecid(current,&sid);
>>>> if (!sid)
>>>> return;
>>>>
>>>> error = security_secid_to_secctx(sid,&ctx,&len);
>>>> if (error) {
>>>> if (error != -EINVAL)
>>>> goto error_path;
>>>> return;
>>>> }
>>>>
>>>> audit_log_format(ab, " subj=%s", ctx);
>>>> security_release_secctx(ctx, len);
>>>> return;
>>>>
>>>> error_path:
>>>> audit_panic("error in audit_log_task_context");
>>>> return;
>>>> }
>>>>
>>>>
>>>> -Roy
>>>>
>>>>
>>>>>>
>>>>>> Signed-off-by: Roy.Li<rongqing.li@windriver.com>
>>>>>> ---
>>>>>> include/linux/security.h | 6 ++++++
>>>>>> security/security.c | 6 ++++++
>>>>>> 2 files changed, 12 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>>>>> index ebd2a53..739ac39 100644
>>>>>> --- a/include/linux/security.h
>>>>>> +++ b/include/linux/security.h
>>>>>> @@ -2560,6 +2560,7 @@ int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
>>>>>> void security_sk_free(struct sock *sk);
>>>>>> void security_sk_clone(const struct sock *sk, struct sock *newsk);
>>>>>> void security_sk_classify_flow(struct sock *sk, struct flowi *fl);
>>>>>> +void security_sk_getsecid(struct sock *sk, u32 *secid);
>>>>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl);
>>>>>> void security_sock_graft(struct sock*sk, struct socket *parent);
>>>>>> int security_inet_conn_request(struct sock *sk,
>>>>>> @@ -2701,6 +2702,11 @@ static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>>>>> {
>>>>>> }
>>>>>>
>>>>>> +static inline void security_sk_getsecid(struct sock *sk, u32 *secid)
>>>>>> +{
>>>>>> + *secid = 0;
>>>>>> +}
>>>>>> +
>>>>>> static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>>>>> {
>>>>>> }
>>>>>> diff --git a/security/security.c b/security/security.c
>>>>>> index 0e4fccf..b0e0825 100644
>>>>>> --- a/security/security.c
>>>>>> +++ b/security/security.c
>>>>>> @@ -1104,6 +1104,12 @@ void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
>>>>>> }
>>>>>> EXPORT_SYMBOL(security_sk_classify_flow);
>>>>>>
>>>>>> +void security_sk_getsecid(struct sock *sk, u32 *secid)
>>>>>> +{
>>>>>> + security_ops->sk_getsecid(sk, secid);
>>>>>> +}
>>>>>> +EXPORT_SYMBOL(security_sk_getsecid);
>>>>>> +
>>>>>> void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
>>>>>> {
>>>>>> security_ops->req_classify_flow(req, fl);
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
--
Best Reagrds,
Roy | RongQing Li
^ permalink raw reply
* Re: [PATCH] gianfar: reduce stack usage in gianfar_ethtool.c
From: Wang Shaoyan @ 2011-08-10 2:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, netdev, davem, Wang Shaoyan
In-Reply-To: <1312908784.2371.53.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
2011/8/10 Eric Dumazet <eric.dumazet@gmail.com>:
> Le mercredi 10 août 2011 à 00:39 +0800, stufever@gmail.com a écrit :
>> From: Wang Shaoyan <wangshaoyan.pt@taobao.com>
>>
>> drivers/net/gianfar_ethtool.c:765: warning: the frame size of 2048 bytes is larger than 1024 bytes
>>
>> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
>> ---
>> drivers/net/gianfar_ethtool.c | 20 +++++++++++++++++---
>> 1 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/gianfar_ethtool.c b/drivers/net/gianfar_ethtool.c
>> index 6e35069..134fe1b 100644
>> --- a/drivers/net/gianfar_ethtool.c
>> +++ b/drivers/net/gianfar_ethtool.c
>> @@ -686,10 +686,21 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
>> {
>> unsigned int last_rule_idx = priv->cur_filer_idx;
>> unsigned int cmp_rqfpr;
>> - unsigned int local_rqfpr[MAX_FILER_IDX + 1];
>> - unsigned int local_rqfcr[MAX_FILER_IDX + 1];
>> + unsigned int *local_rqfpr;
>> + unsigned int *local_rqfcr;
>> int i = 0x0, k = 0x0;
>> int j = MAX_FILER_IDX, l = 0x0;
>> + int ret = 1;
>> +
>> + local_rqfpr = kmalloc(sizeof(unsigned int) * (MAX_FILER_IDX + 1),
>> + GFP_KERNEL);
>> + local_rqfcr = kmalloc(sizeof(unsigned int) * (MAX_FILER_IDX + 1),
>> + GFP_KERNEL);
>> + if (!local_rqfpr || !local_rqfcr) {
>> + pr_err("Out of memory\n");
>
> Please remove this pr_err(), kmalloc() will complain already.
>
>> + ret = 0;
>> + got err;
>> + }
>>
>> switch (class) {
>> case TCP_V4_FLOW:
>> @@ -765,7 +776,10 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
>> priv->cur_filer_idx = priv->cur_filer_idx - 1;
>> }
>>
>> - return 1;
>> +err:
>> + kfree(local_rqfcr);
>> + kfree(local_rqfpr);
>> + return ret;
>> }
>>
>
Thanks, I will do it
> You should now track all "return 0;" done in this function to make sure
> no memory leak is added by your patch.
>
>
>
>
--
Wang Shaoyan
^ permalink raw reply
* Re: [PATCH] gianfar: reduce stack usage in gianfar_ethtool.c
From: Joe Perches @ 2011-08-10 2:28 UTC (permalink / raw)
To: stufever; +Cc: linux-kernel, netdev, davem, Wang Shaoyan
In-Reply-To: <1312943399-14435-1-git-send-email-wangshaoyan.pt@taobao.com>
On Wed, 2011-08-10 at 10:29 +0800, stufever@gmail.com wrote:
> Signed-off-by: Wang Shaoyan <wangshaoyan.pt@taobao.com>
[]
> diff --git a/drivers/net/gianfar_ethtool.c b/drivers/net/gianfar_ethtool.c
[]
> @@ -686,10 +686,21 @@ static int gfar_ethflow_to_filer_table(struct gfar_private *priv, u64 ethflow, u
[]
> + if (!local_rqfpr || !local_rqfcr) {
> + pr_err("Out of memory\n");
> + ret = 0;
> + got err;
"got err" is true.
Please compile test your patches before sending them.
^ 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