Netdev List
 help / color / mirror / Atom feed
* Re: [ethtool PATCH 2/2] Add RX packet classification interface
From: Ben Hutchings @ 2011-03-07 19:00 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Dimitris Michailidis, Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <4D752767.9060205@intel.com>

On Mon, 2011-03-07 at 10:43 -0800, Alexander Duyck wrote:
> On 3/7/2011 10:28 AM, Ben Hutchings wrote:
> > On Mon, 2011-03-07 at 10:22 -0800, Dimitris Michailidis wrote:
> >> Ben Hutchings wrote:
> >>> On Mon, 2011-03-07 at 09:04 -0800, Alexander Duyck wrote:
> >>>> The only time where location really matters is if you are attempting to
> >>>> overwrite an existing rule and I am not sure how that would be handled
> >>>> in ntuple anyway since right now adding additional rules via ntuple for
> >>>> ixgbe just results in duplicate rules being defined.
> >>>
> >>> As I understand it, the location also determines the *priority* for the
> >>> rule.
> >>
> >> This is true, at least for TCAMs.  But it's relevant only when multiple
> >> filters would match a packet.  People often use non-overlapping filters, for
> >> these adding the filter at any available slot is OK.
> >
> > Right.  But ethtool would have to determine that the filter was non-
> > overlapping, before ignoring the location.  Also it cannot allow
> > deletion by location if it ever ignores the location on insertion.  We
> > should make the location optional at both the command-line and API
> > level, but never ignore it.
> >
> 
> I wasn't implying that we ignore it for rules inserted via the nfc 
> interface.  Only for those inserted via the ntuple interface.

We should never fall back to the ntuple interface if a location is
specified!

> My reasoning for that was because it had occurred to me that what my 
> patch series had done is allow for ntuples to be displayed via the 
> get_rx_nfc interface.  As such you would end up with a location being 
> implied when displaying the rules since it would give you a list of n 
> entities.

We need to sort that out then.

> If you attempted to restore the rules you would probably end up with the 
> location information for filters 0..(n-1), and that could be dropped 
> since it would just be extra information.
> 
> >>> Which is why I wrote that "@fs.@location specifies the index to
> >>> use and must not be ignored."
> >>>
> >>> To support hardware where the filter table is hash-based rather than a
> >>> TCAM, we would need some kind of flag or special value of location that
> >>> means 'wherever'.
> >>
> >> I'd find the 'wherever' option useful for TCAMs too.  Maybe even have a few
> >> of those, like 'first available', 'any', and 'last available'.  The last one
> >> is quite useful for catch-all rules without requiring one to know the TCAM size.
> >
> > Agreed.
> >
> > Ben.
> 
> The first and last options make a lot of sense to me.  The one I'm not 
> sure about would be the "any" option.  It seems like it would be 
> redundant with the "first available" option or is there something I'm 
> missing?
> 
> Also the code I have currently for the user space is just starting at 0 
> and filling in the rules on a first available basis for location not 
> specified.  Is this going to work for most cases or should I look at 
> changing it to something like a "last available" approach for the nfc 
> based filters?

My *guess* (and this is just a guess) is that users are more likely to
want to specify explicit priorities for the high-priority rules and not
for the low-priority rules.  So if the location is not explicitly set
then we should choose the last available (lowest-priority) location in a
TCAM, possibly excluding the very last location so that 'last' will
still work.

Ben.

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


^ permalink raw reply

* Re: [ethtool PATCH 2/2] Add RX packet classification interface
From: Dimitris Michailidis @ 2011-03-07 19:11 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Alexander Duyck, Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <1299524407.2522.30.camel@bwh-desktop>

Ben Hutchings wrote:
> On Mon, 2011-03-07 at 10:43 -0800, Alexander Duyck wrote:
>> On 3/7/2011 10:28 AM, Ben Hutchings wrote:
>>> On Mon, 2011-03-07 at 10:22 -0800, Dimitris Michailidis wrote:
>>>> Ben Hutchings wrote:
>>>>> On Mon, 2011-03-07 at 09:04 -0800, Alexander Duyck wrote:
>>>>>> The only time where location really matters is if you are attempting to
>>>>>> overwrite an existing rule and I am not sure how that would be handled
>>>>>> in ntuple anyway since right now adding additional rules via ntuple for
>>>>>> ixgbe just results in duplicate rules being defined.
>>>>> As I understand it, the location also determines the *priority* for the
>>>>> rule.
>>>> This is true, at least for TCAMs.  But it's relevant only when multiple
>>>> filters would match a packet.  People often use non-overlapping filters, for
>>>> these adding the filter at any available slot is OK.
>>> Right.  But ethtool would have to determine that the filter was non-
>>> overlapping, before ignoring the location.  Also it cannot allow
>>> deletion by location if it ever ignores the location on insertion.  We
>>> should make the location optional at both the command-line and API
>>> level, but never ignore it.
>>>
>> I wasn't implying that we ignore it for rules inserted via the nfc 
>> interface.  Only for those inserted via the ntuple interface.
> 
> We should never fall back to the ntuple interface if a location is
> specified!
> 
>> My reasoning for that was because it had occurred to me that what my 
>> patch series had done is allow for ntuples to be displayed via the 
>> get_rx_nfc interface.  As such you would end up with a location being 
>> implied when displaying the rules since it would give you a list of n 
>> entities.
> 
> We need to sort that out then.
> 
>> If you attempted to restore the rules you would probably end up with the 
>> location information for filters 0..(n-1), and that could be dropped 
>> since it would just be extra information.
>>
>>>>> Which is why I wrote that "@fs.@location specifies the index to
>>>>> use and must not be ignored."
>>>>>
>>>>> To support hardware where the filter table is hash-based rather than a
>>>>> TCAM, we would need some kind of flag or special value of location that
>>>>> means 'wherever'.
>>>> I'd find the 'wherever' option useful for TCAMs too.  Maybe even have a few
>>>> of those, like 'first available', 'any', and 'last available'.  The last one
>>>> is quite useful for catch-all rules without requiring one to know the TCAM size.
>>> Agreed.
>>>
>>> Ben.
>> The first and last options make a lot of sense to me.  The one I'm not 
>> sure about would be the "any" option.  It seems like it would be 
>> redundant with the "first available" option or is there something I'm 
>> missing?
>>
>> Also the code I have currently for the user space is just starting at 0 
>> and filling in the rules on a first available basis for location not 
>> specified.  Is this going to work for most cases or should I look at 
>> changing it to something like a "last available" approach for the nfc 
>> based filters?
> 
> My *guess* (and this is just a guess) is that users are more likely to
> want to specify explicit priorities for the high-priority rules and not
> for the low-priority rules.  So if the location is not explicitly set
> then we should choose the last available (lowest-priority) location in a
> TCAM, possibly excluding the very last location so that 'last' will
> still work.

I agree with this (and I misspoke in my previous mail about defaulting to 
first available).  I'll just reiterate that if a user doesn't specify a 
location the driver, rather than ethtool, needs to select one in order to 
accommodate any device restrictions.

^ permalink raw reply

* Re: RFC: dsa slave open handling
From: Peter Korsgaard @ 2011-03-07 19:35 UTC (permalink / raw)
  To: Lennert Buytenhek; +Cc: davem, netdev
In-Reply-To: <20110307182308.GY16649@mail.wantstofly.org>

>>>>> "Lennert" == Lennert Buytenhek <buytenh@wantstofly.org> writes:

Hi,

 >> Currently the state of the dsa master device and slave devices are not
 >> synchronized which leads to various issues. With dsa, the master device
 >> needs to be running before you can use the slave devices, but it
 >> shouldn't be used directly for I/O.

 Lennert> I think this should not be handled any differently than VLAN
 Lennert> interfaces, where eth0 needs to be up before you can exchange
 Lennert> packets over eth0.123.

 Lennert> IOW, if you want to do DHCP over wan (a DSA-tunneled interface
 Lennert> over eth0), then just use an initramfs that sets eth0 up and
 Lennert> then runs a DHCP client on wan.

True. There are a few differences between vlans and dsa devices though,
which might make us want to handle them differently:

- VLANs are configured at runtime from userspace, DSA devices are
  configured directly in kernel code (E.G platform devices) - So by
  definition you cannot use the autconfig / nfsroot stuff on VLANs as
  they are not created before userspace runs, but DSA devices are
  basically indistinguishable from "real" devices from the POV of
  userspace.

- VLANs relation to their upstream ethernet device is clearly visible
  (E.G through brctl), whereas the details about what interface a DSA
  switch is connected to isn't.

-- 
Bye, Peter Korsgaard

^ permalink raw reply

* Re: RFC: dsa slave open handling
From: Lennert Buytenhek @ 2011-03-07 19:38 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: davem, netdev
In-Reply-To: <87wrkalo4s.fsf@macbook.be.48ers.dk>

On Mon, Mar 07, 2011 at 08:35:47PM +0100, Peter Korsgaard wrote:

>  >> Currently the state of the dsa master device and slave devices are not
>  >> synchronized which leads to various issues. With dsa, the master device
>  >> needs to be running before you can use the slave devices, but it
>  >> shouldn't be used directly for I/O.
> 
>  Lennert> I think this should not be handled any differently than VLAN
>  Lennert> interfaces, where eth0 needs to be up before you can exchange
>  Lennert> packets over eth0.123.
> 
>  Lennert> IOW, if you want to do DHCP over wan (a DSA-tunneled interface
>  Lennert> over eth0), then just use an initramfs that sets eth0 up and
>  Lennert> then runs a DHCP client on wan.
> 
> True. There are a few differences between vlans and dsa devices though,
> which might make us want to handle them differently:
> 
> - VLANs are configured at runtime from userspace, DSA devices are
>   configured directly in kernel code (E.G platform devices) - So by
>   definition you cannot use the autconfig / nfsroot stuff on VLANs as
>   they are not created before userspace runs, but DSA devices are
>   basically indistinguishable from "real" devices from the POV of
>   userspace.

Sure.  That doesn't change the argument, though.  In fact, in most
situations, you'll e.g. want to bridge the lan[1-4] interfaces
together, and if you then want to DHCP over that bridge, you cannot
explain that to the kernel autoconfigurator either.


> - VLANs relation to their upstream ethernet device is clearly visible
>   (E.G through brctl), whereas the details about what interface a DSA
>   switch is connected to isn't.

commit c084080151e1de92159f8437fde34b6e5bebe35e
Author: Lennert Buytenhek <buytenh@wantstofly.org>
Date:   Fri Mar 20 09:49:49 2009 +0000

    dsa: set ->iflink on slave interfaces to the ifindex of the parent
    
    ..so that we can parse the DSA topology from 'ip link' output:
    
    1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436 qdisc noqueue
    2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast qlen 1000
    3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast qlen 1000
    4: lan1@eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue
    5: lan2@eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue
    6: lan3@eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue
    7: lan4@eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue
    
    Signed-off-by: Lennert Buytenhek <buytenh@marvell.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>


cheers,
Lennert

^ permalink raw reply

* Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Narendra_K @ 2011-03-07 19:44 UTC (permalink / raw)
  To: a.beregalov, linux-next
  Cc: jbarnes, linux-pci, linux-hotplug, netdev, mjg, Matt_Domsch,
	Charles_Rose, Jordan_Hargrave, Shyam_Iyer, sfr
In-Reply-To: <AANLkTimz662+L6eYJm2gh0NwWOxxzQV3503bcs-GMFGu@mail.gmail.com>

On Mon, Mar 07, 2011 at 11:34:05PM +0530, Alexander Beregalov wrote:
> On 4 March 2011 21:43, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Wed, 2 Mar 2011 22:34:17 +0530
> > <Narendra_K@Dell.com> wrote:
> >
> >> On Wed, Feb 23, 2011 at 06:06:42PM +0530, K, Narendra wrote:
> >> > Hello,
> >> >
> >> > This patch exports ACPI _DSM provided firmware instance number and
> >> > string name to sysfs.
> >> >
> >> > V1 -> V2:
> >> > The attribute 'index' is changed to 'acpi_index' as the semantics of
> >> > SMBIOS provided device type instance and ACPI _DSM provided firmware
> >> > instance number are different.
> >> >
> >> > V2 -> V3:
> >> > Matthew Garrett pointed out that 'sysfs_create_groups' does return an
> >> > error when there are no ACPI _DSM attributes available and because of
> >> > that the fallback to SMBIOS will not happen. As a result SMBIOS provided
> >> > attributes are not created. This version of the patch addresses the issue.
> >> >
> >>
> >> V3 -> V4:
> >> Select NLS if (DMI || ACPI) in drivers/pci/Kconfig to prevent build
> >> breakage from an 'allmodconfig'
> >
> > Applied, fingers crossed this time. :)
> >
> 
> Hi,
> 
> It cannot be compiled if CONFIG_ACPI is not set.
> 
> drivers/pci/pci-label.c: In function 'pci_create_firmware_label_files':
> drivers/pci/pci-label.c:366:2: error: implicit declaration of function
> 'device_has_dsm'

Hello,

Sorry for the inconvenience. Please find the fix here -

From: Narendra K <narendra_k@dell.com>
Subject: [PATCH] Fix compilation error when CONFIG_ACPI unset

This patch fixes compilation error descibed below introduced by
the commit 6058989bad05b82e78baacce69ec14f27a11b5fd

drivers/pci/pci-label.c: In function ‘pci_create_firmware_label_files’:
drivers/pci/pci-label.c:366:2: error: implicit declaration of function ‘device_has_dsm’

Signed-off-by: Narendra K <narendra_k@dell.com>
---
 drivers/pci/pci-label.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index 824e247..82fd73d 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -29,7 +29,9 @@
 #include <linux/nls.h>
 #include <linux/acpi.h>
 #include <linux/pci-acpi.h>
+#ifdef CONFIG_ACPI
 #include <acpi/acpi_drivers.h>
+#endif
 #include <acpi/acpi_bus.h>
 #include "pci.h"
 
@@ -174,6 +176,12 @@ pci_remove_acpi_index_label_files(struct pci_dev *pdev)
 	return -1;
 }
 
+static inline bool
+device_has_dsm(struct device *dev)
+{
+	return false;
+}
+
 #else
 
 static const char device_label_dsm_uuid[] = {
-- 
1.7.3.1

With regards,
Narendra K

^ permalink raw reply related

* Re: Network link detection
From: Dan Williams @ 2011-03-07 19:49 UTC (permalink / raw)
  To: Nico Schümann; +Cc: David Miller, chris.friesen, linux-kernel, netdev
In-Reply-To: <20110303222957.GA30472@svh.nico22.de>

On Thu, 2011-03-03 at 23:29 +0100, Nico Schümann wrote:
> On Thu, Mar 03, 2011 at 02:01:06PM -0800, David Miller wrote:
> > From: Chris Friesen <chris.friesen@genband.com>
> > Date: Thu, 03 Mar 2011 15:38:35 -0600
> > 
> > > You might look at whether you could write a kernel module to register
> > > for NETDEV_CHANGE notifications and pass that back to userspace.
> > 
> > This is the kind of responses you get when you ask networking specific
> > questions and don't CC: netdev :-/
> > 
> 
> Thank you for CC.
> 
> > There is this thing called netlink, you can listen for arbitrary
> > network state change events on a socket, and get the link state
> > notifications you are looking for.  It's in use by many real
> > applications like NetworkManager and co.
> 
> That really looks like what I'm looking for. I was already wondering 
> where NetworkManager gets the link state changes from, but I just 
> expected it to poll. So now I'll read a bit of documentation and 
> hopefully get it work.

http://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/src/nm-netlink-monitor.c

NM uses libnl as the basic library for parsing netlink messages and
handling communication with the kernel.  Which is why you'll see a lot
of nl_* calls in there.  NM sets up the netlink connection using libnl,
then creates a GIOChannel to handle communication over the netlink
socket.  When something comes in (to event_handler()) the code handles
error conditions on the socket, then dispatches to libnl for processing.
libnl then calls back into NM to handle the actual message in
event_msg_ready().

Dan

^ permalink raw reply

* Re: [PATCH 00/17] mlx4_en: driver updates
From: David Miller @ 2011-03-07 19:52 UTC (permalink / raw)
  To: yevgenyp; +Cc: netdev
In-Reply-To: <4D74B092.5010408@mellanox.co.il>

From: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Date: Mon, 7 Mar 2011 12:16:50 +0200

> This patch set presents a set of fixes, performance improvements and new features for the mlx4_core and mlx4_en modules.
> main changes are:
> - MSI-X per ring
> - Changing RX packet steering model for supported devices
> - BlueFlame support for better TX latency.
> - Adaptive moderation improvements.
> 

Please synchronize your changes with me more frequently, so that I don't
have to review so many changes at one time.

^ permalink raw reply

* Re: [Bugme-new] [Bug 30462] New: High cpu usage when someone sends many ipv6 udp packages
From: Ernst Sjöstrand @ 2011-03-07 19:54 UTC (permalink / raw)
  To: David Miller; +Cc: akpm, netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <AANLkTim9FKD7RRi02b+5=gQ85jNybqVtoT_ydW0imNev@mail.gmail.com>

Hi,

seems like reverting "ipv6: Always clone offlink routes." fixes the issue.

Regards
//Ernst

2011/3/5 Ernst Sjöstrand <ernstp@gmail.com>:
> I've only used precompiled kernels so far so I don't have stuff set up to
> compile kernels right now,
> probably won't have time to do that this weekend. Not tonight in GMT+1
> anyway!
>
> Regards
> //Ernst
>
> On Sat, Mar 5, 2011 at 02:21, David Miller <davem@davemloft.net> wrote:
>>
>> From: Andrew Morton <akpm@linux-foundation.org>
>> Date: Fri, 4 Mar 2011 17:15:06 -0800
>>
>> >> The following ipv6 related changes were introduced between -rc2 and
>> >> -rc3 that I
>> >> can see. The "Revert 'administrative down' address handling changes."
>> >> looked
>> >> big...
>> >>       ipv6: Always clone offlink routes.
>> >>       ipv6: Revert 'administrative down' address handling changes.
>>
>> Can we narrow it down to which of those two changes introduced the
>> regression?
>>
>> We have another issue, still open, which is caused by the first
>> change, so maybe try reverting that one first.
>>
>> Thanks.
>
>

^ permalink raw reply

* Re: [patch net-next-2.6 4/8] bonding: wrap slave state work
From: Nicolas de Pesloüan @ 2011-03-07 19:55 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110307095839.GC2832@psychotron.brq.redhat.com>

Le 07/03/2011 10:58, Jiri Pirko a écrit :
>>>
>>> +static inline void bond_set_active_slave(struct slave *slave)
>>> +{
>>> +	slave->backup = 0;
>>
>> In the comment above, you said that the possible value for backup
>> corresponds with BOND_STATE_ACTIVE and BOND_STATE_BACKUP.
>>
>> So, should be:
>>
>> slave->backup = BOND_STATE_ACTIVE;
>>
>>> +}
>>> +
>>> +static inline void bond_set_backup_slave(struct slave *slave)
>>> +{
>>> +	slave->backup = 1;
>>
>> slave->backup = BOND_STATE_BACKUP;
>>
>
> Well, I think it's weird and misleading to assign some define to :1
> bitfield. Should be 0 or 1, nothing else.

Agreed, but the comment appears missleading... May be you should fix the comment, not the code.

	Nicolas.


^ permalink raw reply

* Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Greg KH @ 2011-03-07 19:56 UTC (permalink / raw)
  To: Narendra_K
  Cc: a.beregalov, linux-next, jbarnes, linux-pci, linux-hotplug,
	netdev, mjg, Matt_Domsch, Charles_Rose, Jordan_Hargrave,
	Shyam_Iyer, sfr
In-Reply-To: <20110307200512.GA22450@fedora14-r610.oslab.blr.amer.dell.com>

On Mon, Mar 07, 2011 at 11:44:52AM -0800, Narendra_K@Dell.com wrote:
> --- a/drivers/pci/pci-label.c
> +++ b/drivers/pci/pci-label.c
> @@ -29,7 +29,9 @@
>  #include <linux/nls.h>
>  #include <linux/acpi.h>
>  #include <linux/pci-acpi.h>
> +#ifdef CONFIG_ACPI
>  #include <acpi/acpi_drivers.h>
> +#endif

You should never need a #ifdef in a .c file for an include file.  If so,
something is really wrong.

thanks,

greg k-h

^ permalink raw reply

* [PATCH net-next-2.6 V2] net: enhance the documentation for rx_handler.
From: Nicolas de Pesloüan @ 2011-03-07 20:01 UTC (permalink / raw)
  To: netdev
  Cc: davem, shemminger, eric.dumazet, kaber, fubar, andy,
	Nicolas de Pesloüan
In-Reply-To: <20110307083607.3299ed0c@nehalam>

Signed-off-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
Reviewed-by: Jiri Pirko <jpirko@redhat.com>
---
This apply on top of Jiri's last patch serie, including the last one that
commented the RX_HANDLER_* values.

 include/linux/netdevice.h |   53 ++++++++++++++++++++++++++++++++++++++------
 net/core/dev.c            |    2 +
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 26e03f9..0c9dc93 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -390,15 +390,52 @@ enum gro_result {
 };
 typedef enum gro_result gro_result_t;
 
+/**
+ * enum rx_handler_result - Possible return values for rx_handlers.
+ * @RX_HANDLER_CONSUMED: skb was consumed by rx_handler, do not process it
+ * further.
+ * @RX_HANDLER_ANOTHER: Do another round in receive path. This is indicated in
+ * case skb->dev was changed by rx_handler.
+ * @RX_HANDLER_EXACT: Force exact delivery, no wildcard.
+ * @RX_HANDLER_PASS: Do nothing, process the skb as if no rx_handler was called.
+ *
+ * rx_handlers are functions called from inside __netif_receive_skb(), to do
+ * special processing of the skb, prior to delivery to protocol handlers.
+ *
+ * Currently, a net_device can only have a single rx_handler registered. Trying
+ * to register a second rx_handler will return -EBUSY.
+ *
+ * To register a rx_handler on a net_device, use netdev_rx_handler_register().
+ * To unregister a rx_handler on a net_device, use
+ * netdev_rx_handler_unregister().
+ *
+ * Upon return, rx_handler is expected to tell __netif_receive_skb() what to
+ * do with the skb.
+ *
+ * If the rx_handler consumed to skb in some way, it should return
+ * RX_HANDLER_CONSUMED. This is appropriate when the rx_handler arranged for
+ * the skb to be delivered in some other ways.
+ *
+ * If the rx_handler changed skb->dev, to divert the skb to another
+ * net_device, it should return RX_HANDLER_ANOTHER. The rx_handler for the
+ * new device will be called if it exists.
+ *
+ * If the rx_handler consider the skb should be ignored, it should return
+ * RX_HANDLER_EXACT. The skb will only be delivered to protocol handlers that
+ * are registred on exact device (ptype->dev == skb->dev).
+ *
+ * If the rx_handler didn't changed skb->dev, but want the skb to be normally 
+ * delivered, it should return RX_HANDLER_PASS.
+ * 
+ * A device without a registered rx_handler will behave as if rx_handler
+ * returned RX_HANDLER_PASS.
+ */
+
 enum rx_handler_result {
-	RX_HANDLER_CONSUMED,	/* skb was consumed by rx_handler,
-				   do not process it further. */
-	RX_HANDLER_ANOTHER,	/* Do another round in receive path.
-				   This is indicated in case skb->dev
-				   was changed by rx_handler */
-	RX_HANDLER_EXACT,	/* Force exact delivery, no wildcard */
-	RX_HANDLER_PASS,	/* Do nothing, pass the skb as if
-				   no rx_handler was called */
+	RX_HANDLER_CONSUMED,
+	RX_HANDLER_ANOTHER,	
+	RX_HANDLER_EXACT,
+	RX_HANDLER_PASS,
 };
 typedef enum rx_handler_result rx_handler_result_t;
 typedef rx_handler_result_t rx_handler_func_t(struct sk_buff **pskb);
diff --git a/net/core/dev.c b/net/core/dev.c
index a368223..3630722 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3060,6 +3060,8 @@ out:
  *	on a failure.
  *
  *	The caller must hold the rtnl_mutex.
+ *
+ *	For a general description of rx_handler, see enum rx_handler_result.
  */
 int netdev_rx_handler_register(struct net_device *dev,
 			       rx_handler_func_t *rx_handler,
-- 
1.7.2.3


^ permalink raw reply related

* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Nicolas de Pesloüan @ 2011-03-07 20:12 UTC (permalink / raw)
  To: Andy Gospodarek
  Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet
In-Reply-To: <20110307143202.GS11864@gospo.rdu.redhat.com>

Le 07/03/2011 15:32, Andy Gospodarek a écrit :
> On Mon, Mar 07, 2011 at 01:51:00PM +0100, Jiri Pirko wrote:
>> Recent patch "bonding: move processing of recv handlers into
>> handle_frame()" caused a regression on following net scheme:
>>
>> eth0 - bond0 - bond0.5
>>
>> where arp monitoring is happening over vlan. This patch fixes it by
>> reinjecting the arp packet into bonding slave device so the bonding
>> rx_handler can pickup and process it.
>>
>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

> Thanks, Jiri.  I will test and make sure it works correctly now.  I know
> several users who would be quite disappointed if this feature was
> removed.

Andy, while you are testing it, can you ensure it also works for the following setup?

eth0 -> bond0 -> br0 -> br0.100

I think it should, as long as the slave is the first device in the path.

Can you imagine a bonding setup where the slaves are not first in the path?

eth0 -> br0 -> bond0?
eth0 -> eth1(macvlan) -> bond0?
eth0 -> eth0.100 -> bond0?

	Nicolas.

^ permalink raw reply

* Re: [PATCH 1/8] Phonet: fix NULL-deref in previous patch series
From: David Miller @ 2011-03-07 20:23 UTC (permalink / raw)
  To: remi.denis-courmont; +Cc: netdev
In-Reply-To: <1299232831-3132-1-git-send-email-remi.denis-courmont@nokia.com>

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Fri,  4 Mar 2011 12:00:24 +0200

> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

What "previous patch series"?

Nobody looking at this patch in isoluation will understand
what you're talking about.

Reference other changes like a man, by mentioned the SHA1 ID and
giving the commit message header line inside of ("") right
afterwards. :-)



^ permalink raw reply

* Re: [PATCH 3/8] Phonet: fix backlog callback return value
From: David Miller @ 2011-03-07 20:23 UTC (permalink / raw)
  To: remi.denis-courmont; +Cc: netdev
In-Reply-To: <1299232831-3132-3-git-send-email-remi.denis-courmont@nokia.com>


More verbose commit message please.

What was wrong with the old error values and what exactly do
you claim to be doing to correct them?

If all I get is single header line as a commit message, you're not
doing due dilligence and you're making your work harder to
review and understand for other people.

^ permalink raw reply

* Re: [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: David Miller @ 2011-03-07 20:27 UTC (permalink / raw)
  To: nhorman; +Cc: netdev, rds-devel, andy.grover
In-Reply-To: <1299083302-5168-1-git-send-email-nhorman@tuxdriver.com>

From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed,  2 Mar 2011 11:28:22 -0500

> Recently had this bug halt reported to me:

Well, does anyone on the RDS team care about this bug fix at all?

Stuff like this should not sit for nearly a week without any reply
whatsoever.

^ permalink raw reply

* Re: [PATCH 3/8] Phonet: fix backlog callback return value
From: Rémi Denis-Courmont @ 2011-03-07 20:30 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20110307.122329.39193888.davem@davemloft.net>

Le lundi 7 mars 2011 22:23:29 David Miller, vous avez écrit :
> More verbose commit message please.
> 
> What was wrong with the old error values and what exactly do
> you claim to be doing to correct them?

They were returning -Exxx errno's where the caller expected NET_RX_* values.
I can "fix" it but it seemed obvious.

-- 
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis

^ permalink raw reply

* Re: [Bugme-new] [Bug 30092] New: smsc911x.c drops long packets with VLAN tags
From: David Miller @ 2011-03-07 20:31 UTC (permalink / raw)
  To: weinholt; +Cc: akpm, netdev, bugzilla-daemon, bugme-daemon, steve.glendinning
In-Reply-To: <lnsjv58vl2.fsf@desktop3.csbnet.se>

From: weinholt@csbnet.se (Göran Weinholt)
Date: Wed, 02 Mar 2011 15:07:21 +0100

> Subject: [PATCH] net/smsc911x.c: Set the VLAN1 register to fix VLAN MTU problem
> From: Göran Weinholt <weinholt@csbnet.se>
> 
> The smsc911x driver would drop frames longer than 1518 bytes, which is a
> problem for networks with VLAN tagging. The VLAN1 tag register is used
> to increase the legal frame size to 1522 when a VLAN tag is identified.
> 
> Signed-off-by: Göran Weinholt <weinholt@csbnet.se>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 3/8] Phonet: fix backlog callback return value
From: David Miller @ 2011-03-07 20:31 UTC (permalink / raw)
  To: remi; +Cc: netdev
In-Reply-To: <201103072230.14080.remi@remlab.net>

From: "Rémi Denis-Courmont" <remi@remlab.net>
Date: Mon, 7 Mar 2011 22:30:13 +0200

> Le lundi 7 mars 2011 22:23:29 David Miller, vous avez écrit :
>> More verbose commit message please.
>> 
>> What was wrong with the old error values and what exactly do
>> you claim to be doing to correct them?
> 
> They were returning -Exxx errno's where the caller expected NET_RX_* values.

Then say that in the commit message.


^ permalink raw reply

* Re: [PATCH 1/8] Phonet: fix NULL-deref in previous patch series
From: Rémi Denis-Courmont @ 2011-03-07 20:33 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20110307.122327.183060932.davem@davemloft.net>

Le lundi 7 mars 2011 22:23:27 David Miller, vous avez écrit :
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> Date: Fri,  4 Mar 2011 12:00:24 +0200
> 
> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> 
> What "previous patch series"?

The one you complained about applying patches manually, so nothing "stable". 
Though that's not an excuse for OOPSing nor being terse.

-- 
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: harmonize the call to ptype_all and ptype_base handlers.
From: Nicolas de Pesloüan @ 2011-03-07 20:41 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Nicolas de Pesloüan, netdev, davem, shemminger, eric.dumazet,
	kaber, fubar, andy
In-Reply-To: <20110307100318.GD2832@psychotron.brq.redhat.com>

Le 07/03/2011 11:03, Jiri Pirko a écrit :
> Sun, Mar 06, 2011 at 02:25:16PM CET, nicolas.2p.debian@free.fr wrote:
>> Until now, ptype_all and ptype_base delivery in __netif_receive_skb() is
>> inconsistent.
>>
>> - For ptype_all, we deliver to every device crossed while walking the
>> rx_handler path (inside the another_round loop), and there is no way to stop
>> wildcard delivery (no exact match logic).
>> - For ptype_base, we deliver to the lowest device (orig_dev) and to the highest
>> (skb->dev) and we can ask for exact match delivery.
>>
>> This patch try and fix this, by:
>>
>> 1/ Doing exact match delivery for both ptype_all and ptype_base, while walking
>>    the rx_handler path.
>> 2/ Doing wildcard match delivery at the end of __netif_receive_skb(), if not
>>    asked to do exact match delivery only.
>>
>> Signed-off-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>> ---
>>
>> This apply on top of the last batch of patch from Jiri Pirko.
>> ---
>> net/core/dev.c |   32 ++++++++++++++++++++++++--------
>> 1 files changed, 24 insertions(+), 8 deletions(-)
>>
>
> I tend to like this patch. However I'm not sure if extra 2 loops don't
> introduce noticable overhead :/

I think ptype_all and ptype_base lists should only contain entries having ptype->dev == NULL.

The entries having ptype->dev != NULL should be on per net_device lists. The head of those lists 
could/should be in a ptype_all and a ptype_base property in net_device.

This would speed up the exact-match loops, because they would scan small (or empty) lists.

I need to double check the possible impact of this proposal.

	Nicolas.

^ permalink raw reply

* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: Chris Mason @ 2011-03-07 20:41 UTC (permalink / raw)
  To: David Miller; +Cc: nhorman, netdev, rds-devel, Venkat Venkatsubra
In-Reply-To: <20110307.122753.59686845.davem@davemloft.net>

Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Wed,  2 Mar 2011 11:28:22 -0500
> 
> > Recently had this bug halt reported to me:
> 
> Well, does anyone on the RDS team care about this bug fix at all?
> 
> Stuff like this should not sit for nearly a week without any reply
> whatsoever.
> 

The patch looks good to me, but I'm surprised we haven't seen it here.
Venkat please take a look (link to the patch below)  Has it only been seen on ppc?

http://permalink.gmane.org/gmane.linux.network/187933

-chris

^ permalink raw reply

* Re: [PATCH] bridge:  control carrier based on ports online
From: Nicolas de Pesloüan @ 2011-03-07 20:48 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David S. Miller, Adam Majer, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, bridge, netdev, Andy Gospodarek, Jay Vosburgh
In-Reply-To: <20110307103406.27330529@nehalam>

Le 07/03/2011 19:34, Stephen Hemminger a écrit :
> This makes the bridge device behave like a physical device.
> In earlier releases the bridge always asserted carrier. This
> changes the behavior so that bridge device carrier is on only
> if one or more ports are in the forwarding state. This
> should help IPv6 autoconfiguration, DHCP, and routing daemons.
>
> I did brief testing with Network and Virt manager and they
> seem fine, but since this changes behavior of bridge, it should
> wait until net-next (2.6.39).
>
> Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
>
> ---
>   net/bridge/br_device.c    |    4 ++++
>   net/bridge/br_stp.c       |   35 ++++++++++++++++++++++-------------
>   net/bridge/br_stp_timer.c |    1 +
>   3 files changed, 27 insertions(+), 13 deletions(-)
>
> --- a/net/bridge/br_device.c	2011-03-07 08:40:08.913599513 -0800
> +++ b/net/bridge/br_device.c	2011-03-07 08:40:48.382377389 -0800
> @@ -78,6 +78,8 @@ static int br_dev_open(struct net_device
>   {
>   	struct net_bridge *br = netdev_priv(dev);
>
> +	netif_carrier_off(dev);
> +
>   	br_features_recompute(br);
>   	netif_start_queue(dev);
>   	br_stp_enable_bridge(br);
> @@ -94,6 +96,8 @@ static int br_dev_stop(struct net_device
>   {
>   	struct net_bridge *br = netdev_priv(dev);
>
> +	netif_carrier_off(dev);
> +
>   	br_stp_disable_bridge(br);
>   	br_multicast_stop(br);
>
> --- a/net/bridge/br_stp.c	2011-03-07 08:41:58.619783678 -0800
> +++ b/net/bridge/br_stp.c	2011-03-07 08:53:58.953558810 -0800
> @@ -397,28 +397,37 @@ static void br_make_forwarding(struct ne
>   void br_port_state_selection(struct net_bridge *br)
>   {
>   	struct net_bridge_port *p;
> +	unsigned int liveports = 0;
>
>   	/* Don't change port states if userspace is handling STP */
>   	if (br->stp_enabled == BR_USER_STP)
>   		return;
>
>   	list_for_each_entry(p,&br->port_list, list) {
> -		if (p->state != BR_STATE_DISABLED) {
> -			if (p->port_no == br->root_port) {
> -				p->config_pending = 0;
> -				p->topology_change_ack = 0;
> -				br_make_forwarding(p);
> -			} else if (br_is_designated_port(p)) {
> -				del_timer(&p->message_age_timer);
> -				br_make_forwarding(p);
> -			} else {
> -				p->config_pending = 0;
> -				p->topology_change_ack = 0;
> -				br_make_blocking(p);
> -			}
> +		if (p->state == BR_STATE_DISABLED)
> +			continue;
> +
> +		if (p->port_no == br->root_port) {
> +			p->config_pending = 0;
> +			p->topology_change_ack = 0;
> +			br_make_forwarding(p);
> +		} else if (br_is_designated_port(p)) {
> +			del_timer(&p->message_age_timer);
> +			br_make_forwarding(p);
> +		} else {
> +			p->config_pending = 0;
> +			p->topology_change_ack = 0;
> +			br_make_blocking(p);

Is the above part really related to the purpose of this patch? It looks like (good) cleanup, but 
should be in a different patch.

Except from this comment,

Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>

>   		}
>
> +		if (p->state == BR_STATE_FORWARDING)
> +			++liveports;
>   	}
> +
> +	if (liveports == 0)
> +		netif_carrier_off(br->dev);
> +	else
> +		netif_carrier_on(br->dev);
>   }
>
>   /* called under bridge lock */
> --- a/net/bridge/br_stp_timer.c	2011-03-07 08:53:25.728770710 -0800
> +++ b/net/bridge/br_stp_timer.c	2011-03-07 08:53:40.273116636 -0800
> @@ -94,6 +94,7 @@ static void br_forward_delay_timer_expir
>   		p->state = BR_STATE_FORWARDING;
>   		if (br_is_designated_for_some_port(br))
>   			br_topology_change_detection(br);
> +		netif_carrier_on(br->dev);
>   	}
>   	br_log_state(p);
>   	spin_unlock(&br->lock);
>


^ permalink raw reply

* Re: [PATCH] net: Enter net/ipv6/ even if CONFIG_IPV6=n
From: David Miller @ 2011-03-07 20:51 UTC (permalink / raw)
  To: randy.dunlap; +Cc: kaber, sfr, netdev, linux-next
In-Reply-To: <4D711B02.2060200@oracle.com>

From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Fri, 04 Mar 2011 09:01:54 -0800

> On 03/04/11 01:35, Thomas Graf wrote:
>> exthdrs_core.c and addrconf_core.c in net/ipv6/ contain bits which
>> must be made available even if IPv6 is disabled.
>> 
>> net/ipv6/Makefile already correctly includes them if CONFIG_IPV6=n
>> but net/Makefile prevents entering the subdirectory.
>> 
>> Signed-off-by: Thomas Graf <tgraf@infradead.org>
> 
> Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Narendra_K @ 2011-03-07 20:55 UTC (permalink / raw)
  To: greg
  Cc: a.beregalov, linux-next, jbarnes, linux-pci, linux-hotplug,
	netdev, mjg, Matt_Domsch, Charles_Rose, Jordan_Hargrave,
	Shyam_Iyer, sfr
In-Reply-To: <20110307195616.GA14888@kroah.com>

On Tue, Mar 08, 2011 at 01:26:16AM +0530, Greg KH wrote:
> On Mon, Mar 07, 2011 at 11:44:52AM -0800, Narendra_K@Dell.com wrote:
> > --- a/drivers/pci/pci-label.c
> > +++ b/drivers/pci/pci-label.c
> > @@ -29,7 +29,9 @@
> >  #include <linux/nls.h>
> >  #include <linux/acpi.h>
> >  #include <linux/pci-acpi.h>
> > +#ifdef CONFIG_ACPI
> >  #include <acpi/acpi_drivers.h>
> > +#endif
> 
> You should never need a #ifdef in a .c file for an include file.  If so,
> something is really wrong.

I agree. Also, i realized that the include was not required to address the
reported error. Please find the revised patch here.

From: Narendra K <narendra_k@dell.com>
Subject: [PATCH] Fix compilation error when CONFIG_ACPI is unset

This patch fixes compilation error descibed below introduced by
the commit 6058989bad05b82e78baacce69ec14f27a11b5fd

drivers/pci/pci-label.c: In function ‘pci_create_firmware_label_files’:
drivers/pci/pci-label.c:366:2: error: implicit declaration of function ‘device_has_dsm’

Signed-off-by: Narendra K <narendra_k@dell.com>
---
 drivers/pci/pci-label.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index 824e247..8c80138 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -174,6 +174,12 @@ pci_remove_acpi_index_label_files(struct pci_dev *pdev)
 	return -1;
 }
 
+static inline bool
+device_has_dsm(struct device *dev)
+{
+	return false;
+}
+
 #else
 
 static const char device_label_dsm_uuid[] = {
-- 
1.7.3.1

With regards,
Narendra K

^ permalink raw reply related

* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: Neil Horman @ 2011-03-07 20:56 UTC (permalink / raw)
  To: Chris Mason; +Cc: David Miller, netdev, rds-devel, Venkat Venkatsubra
In-Reply-To: <1299530363-sup-8922@think>

On Mon, Mar 07, 2011 at 03:41:04PM -0500, Chris Mason wrote:
> Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
> > From: Neil Horman <nhorman@tuxdriver.com>
> > Date: Wed,  2 Mar 2011 11:28:22 -0500
> > 
> > > Recently had this bug halt reported to me:
> > 
> > Well, does anyone on the RDS team care about this bug fix at all?
> > 
> > Stuff like this should not sit for nearly a week without any reply
> > whatsoever.
> > 
> 
> The patch looks good to me, but I'm surprised we haven't seen it here.
> Venkat please take a look (link to the patch below)  Has it only been seen on ppc?
> 
Yes, its only been observed on ppc64.  Its does seem like it should be
observable on other arches.  I presumed it had something to do with ppc64
alignment and how it filled in the sg array, that led to a leftover bit of space
in the scattergather array.  Honestly though I wasnt too worried about ferreting
out the source, since it seemed apparent to me that buffers with
RDS_FLAG_CONG_BITMAP set were just causing rds_[loop|ib]_xmit to return a dummy
positive value (I say dummy because theres no actual data transmitted).  So I
figured if the value is just there to satisfy the return code, it might as well
also satisfy what is otherwise a valid BUG_ON check too.

Best
Neil

> http://permalink.gmane.org/gmane.linux.network/187933
> 
> -chris
> --
> 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


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