* 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: [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: 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: [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: 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 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 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
* [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 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
* 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: [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 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: 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 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: 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: 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: [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: [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 18:57 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Ben Hutchings, Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <4D752767.9060205@intel.com>
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.
>
> 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.
>
> 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?
Not really. I offered it because we were talking about 'wherever' options.
I too find 'first' and 'last' to be most useful.
> 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?
Oh, I hadn't noticed that user space is trying to allocate the index if
unspecified. In my case, some filters need to have special indices, eg
multiple of 4. User space wouldn't know this. I think user space needs to
let the driver allocate indices if the user didn't select one. I'd default
to first available.
^ permalink raw reply
* Re: [Patch] bonding: fix netpoll in active-backup mode
From: Neil Horman @ 2011-03-07 18:50 UTC (permalink / raw)
To: Amerigo Wang
Cc: linux-kernel, Jay Vosburgh, David S. Miller, Herbert Xu,
Paul E. McKenney, John W. Linville, Eric Dumazet, netdev
In-Reply-To: <1299507114-12144-1-git-send-email-amwang@redhat.com>
On Mon, Mar 07, 2011 at 10:11:50PM +0800, Amerigo Wang wrote:
> netconsole doesn't work in active-backup mode, because we don't do anything
> for nic failover in active-backup mode. This patch fixes the problem by:
>
> 1) make slave_enable_netpoll() and slave_disable_netpoll() callable in softirq
> context, that is, moving code after synchronize_rcu_bh() into call_rcu_bh()
> callback function, teaching kzalloc() to use GFP_ATOMIC.
>
> 2) disable netpoll on old slave and enable netpoll on the new slave.
>
> Tested by ifdown the current active slave and ifup it again for several times,
> netconsole works well.
>
> Signed-off-by: WANG Cong <amwang@redhat.com>
>
I may be missing soething but this seems way over-complicated to me. I presume
the problem is that in active backup mode a failover results in the new active
slave not having netpoll setup on it? If thats the case, why not just setup
netpoll on all slaves when ndo_netpoll_setup is called on the bonding interface?
I don't see anything immeidately catastrophic that would happen as a result.
And then you wouldn't have to worry about disabling/enabling anything on a
failover (or during a panic for that matter). As for the rcu bits? Why are
they needed? One would presume that wouldn't (or at least shouldn't) be able to
teardown our netpoll setup until such time as all the pending frames for that
netpoll client have been transmitted. If we're not blocknig on that RCU isn't
really going to help. Seems like the proper fix is take a reference to the
appropriate npinfo struct in netpoll_send_skb, and drop it from the skbs
destructor or some such.
Neil
> ---
> drivers/net/bonding/bond_main.c | 236 +++++++++++++++++++++------------------
> include/linux/netpoll.h | 2 +
> net/core/netpoll.c | 22 ++--
> 3 files changed, 140 insertions(+), 120 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 0592e6d..2d6ec1b 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -907,6 +907,121 @@ static void bond_mc_list_flush(struct net_device *bond_dev,
> }
> }
>
> +/*--------------------------- Netpoll code ---------------------------*/
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +static inline int slave_enable_netpoll(struct slave *slave)
> +{
> + struct netpoll *np;
> + int err = 0;
> +
> + np = kzalloc(sizeof(*np), GFP_ATOMIC);
> + err = -ENOMEM;
> + if (!np)
> + goto out;
> +
> + np->dev = slave->dev;
> + err = __netpoll_setup(np);
> + if (err) {
> + kfree(np);
> + goto out;
> + }
> + slave->np = np;
> +out:
> + return err;
> +}
> +static void slave_netpoll_reclaim(struct rcu_head *rp)
> +{
> + struct netpoll *np = container_of(rp, struct netpoll, rcu);
> + __netpoll_cleanup(np);
> + kfree(np);
> +}
> +static inline void slave_disable_netpoll(struct slave *slave)
> +{
> + struct netpoll *np = slave->np;
> +
> + if (!np)
> + return;
> +
> + slave->np = NULL;
> + call_rcu_bh(&np->rcu, slave_netpoll_reclaim);
> +}
> +static inline bool slave_dev_support_netpoll(struct net_device *slave_dev)
> +{
> + if (slave_dev->priv_flags & IFF_DISABLE_NETPOLL)
> + return false;
> + if (!slave_dev->netdev_ops->ndo_poll_controller)
> + return false;
> + return true;
> +}
> +
> +static void bond_poll_controller(struct net_device *bond_dev)
> +{
> +}
> +
> +static void __bond_netpoll_cleanup(struct bonding *bond)
> +{
> + struct slave *slave;
> + int i;
> +
> + bond_for_each_slave(bond, slave, i)
> + if (IS_UP(slave->dev))
> + slave_disable_netpoll(slave);
> +}
> +static void bond_netpoll_cleanup(struct net_device *bond_dev)
> +{
> + struct bonding *bond = netdev_priv(bond_dev);
> +
> + read_lock(&bond->lock);
> + __bond_netpoll_cleanup(bond);
> + read_unlock(&bond->lock);
> +}
> +
> +static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> +{
> + struct bonding *bond = netdev_priv(dev);
> + struct slave *slave;
> + int i, err = 0;
> +
> + read_lock(&bond->lock);
> + bond_for_each_slave(bond, slave, i) {
> + if (!IS_UP(slave->dev))
> + continue;
> + err = slave_enable_netpoll(slave);
> + if (err) {
> + __bond_netpoll_cleanup(bond);
> + break;
> + }
> + }
> + read_unlock(&bond->lock);
> + return err;
> +}
> +
> +static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> +{
> + return bond->dev->npinfo;
> +}
> +
> +#else
> +static inline int slave_enable_netpoll(struct slave *slave)
> +{
> + return 0;
> +}
> +static inline void slave_disable_netpoll(struct slave *slave)
> +{
> +}
> +static void bond_netpoll_cleanup(struct net_device *bond_dev)
> +{
> +}
> +static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> +{
> + return 0;
> +}
> +static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> +{
> + return NULL;
> +}
> +#endif
> +
> /*--------------------------- Active slave change ---------------------------*/
>
> /*
> @@ -1155,11 +1270,20 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> }
>
> if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
> - if (old_active)
> + if (old_active) {
> bond_set_slave_inactive_flags(old_active);
> + if (bond_netpoll_info(bond))
> + slave_disable_netpoll(old_active);
> + }
>
> if (new_active) {
> + struct netpoll_info *ni;
> bond_set_slave_active_flags(new_active);
> + ni = bond_netpoll_info(bond);
> + if (ni) {
> + new_active->dev->npinfo = ni;
> + slave_enable_netpoll(new_active);
> + }
>
> if (bond->params.fail_over_mac)
> bond_do_fail_over_mac(bond, new_active,
> @@ -1280,116 +1404,6 @@ static void bond_detach_slave(struct bonding *bond, struct slave *slave)
> bond->slave_cnt--;
> }
>
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> -static inline int slave_enable_netpoll(struct slave *slave)
> -{
> - struct netpoll *np;
> - int err = 0;
> -
> - np = kzalloc(sizeof(*np), GFP_KERNEL);
> - err = -ENOMEM;
> - if (!np)
> - goto out;
> -
> - np->dev = slave->dev;
> - err = __netpoll_setup(np);
> - if (err) {
> - kfree(np);
> - goto out;
> - }
> - slave->np = np;
> -out:
> - return err;
> -}
> -static inline void slave_disable_netpoll(struct slave *slave)
> -{
> - struct netpoll *np = slave->np;
> -
> - if (!np)
> - return;
> -
> - slave->np = NULL;
> - synchronize_rcu_bh();
> - __netpoll_cleanup(np);
> - kfree(np);
> -}
> -static inline bool slave_dev_support_netpoll(struct net_device *slave_dev)
> -{
> - if (slave_dev->priv_flags & IFF_DISABLE_NETPOLL)
> - return false;
> - if (!slave_dev->netdev_ops->ndo_poll_controller)
> - return false;
> - return true;
> -}
> -
> -static void bond_poll_controller(struct net_device *bond_dev)
> -{
> -}
> -
> -static void __bond_netpoll_cleanup(struct bonding *bond)
> -{
> - struct slave *slave;
> - int i;
> -
> - bond_for_each_slave(bond, slave, i)
> - if (IS_UP(slave->dev))
> - slave_disable_netpoll(slave);
> -}
> -static void bond_netpoll_cleanup(struct net_device *bond_dev)
> -{
> - struct bonding *bond = netdev_priv(bond_dev);
> -
> - read_lock(&bond->lock);
> - __bond_netpoll_cleanup(bond);
> - read_unlock(&bond->lock);
> -}
> -
> -static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> -{
> - struct bonding *bond = netdev_priv(dev);
> - struct slave *slave;
> - int i, err = 0;
> -
> - read_lock(&bond->lock);
> - bond_for_each_slave(bond, slave, i) {
> - if (!IS_UP(slave->dev))
> - continue;
> - err = slave_enable_netpoll(slave);
> - if (err) {
> - __bond_netpoll_cleanup(bond);
> - break;
> - }
> - }
> - read_unlock(&bond->lock);
> - return err;
> -}
> -
> -static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> -{
> - return bond->dev->npinfo;
> -}
> -
> -#else
> -static inline int slave_enable_netpoll(struct slave *slave)
> -{
> - return 0;
> -}
> -static inline void slave_disable_netpoll(struct slave *slave)
> -{
> -}
> -static void bond_netpoll_cleanup(struct net_device *bond_dev)
> -{
> -}
> -static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> -{
> - return 0;
> -}
> -static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> -{
> - return NULL;
> -}
> -#endif
> -
> /*---------------------------------- IOCTL ----------------------------------*/
>
> static int bond_sethwaddr(struct net_device *bond_dev,
> diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
> index 79358bb..9412aa5 100644
> --- a/include/linux/netpoll.h
> +++ b/include/linux/netpoll.h
> @@ -23,6 +23,7 @@ struct netpoll {
> u8 remote_mac[ETH_ALEN];
>
> struct list_head rx; /* rx_np list element */
> + struct rcu_head rcu;
> };
>
> struct netpoll_info {
> @@ -38,6 +39,7 @@ struct netpoll_info {
> struct delayed_work tx_work;
>
> struct netpoll *netpoll;
> + struct rcu_head rcu;
> };
>
> void netpoll_poll_dev(struct net_device *dev);
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 06be243..9870dac 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -901,6 +901,18 @@ static int __init netpoll_init(void)
> }
> core_initcall(netpoll_init);
>
> +static void netpoll_reclaim(struct rcu_head *rp)
> +{
> + struct netpoll_info *npinfo = container_of(rp, struct netpoll_info, rcu);
> + skb_queue_purge(&npinfo->arp_tx);
> + skb_queue_purge(&npinfo->txq);
> + cancel_delayed_work_sync(&npinfo->tx_work);
> +
> + /* clean after last, unfinished work */
> + __skb_queue_purge(&npinfo->txq);
> + kfree(npinfo);
> +}
> +
> void __netpoll_cleanup(struct netpoll *np)
> {
> struct netpoll_info *npinfo;
> @@ -928,15 +940,7 @@ void __netpoll_cleanup(struct netpoll *np)
> rcu_assign_pointer(np->dev->npinfo, NULL);
>
> /* avoid racing with NAPI reading npinfo */
> - synchronize_rcu_bh();
> -
> - skb_queue_purge(&npinfo->arp_tx);
> - skb_queue_purge(&npinfo->txq);
> - cancel_delayed_work_sync(&npinfo->tx_work);
> -
> - /* clean after last, unfinished work */
> - __skb_queue_purge(&npinfo->txq);
> - kfree(npinfo);
> + call_rcu_bh(&npinfo->rcu, netpoll_reclaim);
> }
> }
> EXPORT_SYMBOL_GPL(__netpoll_cleanup);
>
^ permalink raw reply
* Re: [ethtool PATCH 2/2] Add RX packet classification interface
From: Alexander Duyck @ 2011-03-07 18:43 UTC (permalink / raw)
To: Ben Hutchings
Cc: Dimitris Michailidis, Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <1299522481.2522.24.camel@bwh-desktop>
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.
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.
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?
Thanks,
Alex
^ permalink raw reply
* [PATCH] bridge: control carrier based on ports online
From: Stephen Hemminger @ 2011-03-07 18:34 UTC (permalink / raw)
To: David S. Miller
Cc: Nicolas de Pesloüan, Adam Majer, Alexey Kuznetsov,
Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, bridge, netdev, Andy Gospodarek, Jay Vosburgh
In-Reply-To: <4D748CC3.8060603@gmail.com>
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);
}
+ 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: [ethtool PATCH 2/2] Add RX packet classification interface
From: Ben Hutchings @ 2011-03-07 18:28 UTC (permalink / raw)
To: Dimitris Michailidis
Cc: Alexander Duyck, Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <4D75225B.3010008@chelsio.com>
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.
> > 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.
--
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: [PATCH] dsa/mv88e6060: support nonzero mii base address
From: Lennert Buytenhek @ 2011-03-07 18:24 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: davem, netdev
In-Reply-To: <1299512987-11152-1-git-send-email-jacmet@sunsite.dk>
On Mon, Mar 07, 2011 at 04:49:47PM +0100, Peter Korsgaard wrote:
> The mv88e6060 uses either the lower 16 or upper 16 mii addresses,
> depending on the value of the EE_CLK/ADDR4 pin. Support both
> configurations by using the sw_addr setting as base address.
>
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
Looks good!
Acked-by: Lennert Buytenhek <buytenh@wantstofly.org>
^ permalink raw reply
* Re: RFC: dsa slave open handling
From: Lennert Buytenhek @ 2011-03-07 18:23 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: davem, netdev
In-Reply-To: <871v2inajz.fsf@macbook.be.48ers.dk>
On Mon, Mar 07, 2011 at 05:46:08PM +0100, Peter Korsgaard wrote:
> Hi,
Hello,
> 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.
I think this should not be handled any differently than VLAN interfaces,
where eth0 needs to be up before you can exchange packets over eth0.123.
IOW, if you want to do DHCP over wan (a DSA-tunneled interface over
eth0), then just use an initramfs that sets eth0 up and then runs a
DHCP client on wan.
The fact that in-kernel IP autoconfiguration can't deal with DSA just
means that you shouldn't use the in-kernel IP autoconfiguration -- you
could "fix" this with one of your proposed patches, but then the next
thing will be someone wanting to do in-kernel IP autoconfig over a VLAN
interface stacked onto a DSA interface, etc, which then still wouldn't
work.
cheers,
Lennert
^ 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