* Re: [V2 Patch] bonding: move procfs code into bond_procfs.c
From: Andy Gospodarek @ 2011-03-07 16:17 UTC (permalink / raw)
To: Amerigo Wang; +Cc: linux-kernel, davem, Jay Vosburgh, netdev
In-Reply-To: <1299484726-29905-1-git-send-email-amwang@redhat.com>
On Mon, Mar 07, 2011 at 03:58:46PM +0800, Amerigo Wang wrote:
> V2: Move #ifdef CONFIG_PROC_FS into bonding.h, as suggested by David.
>
> bond_main.c is bloating, separate the procfs code out,
> move them to bond_procfs.c
>
> Signed-off-by: WANG Cong <amwang@redhat.com>
>
Looks good and works as expected.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: enhance the documentation for rx_handler.
From: Stephen Hemminger @ 2011-03-07 16:36 UTC (permalink / raw)
To: Nicolas de Pesloüan; +Cc: netdev, davem, eric.dumazet, kaber, fubar, andy
In-Reply-To: <1299441608-25482-1-git-send-email-nicolas.2p.debian@free.fr>
On Sun, 6 Mar 2011 21:00:08 +0100
Nicolas de Pesloüan <nicolas.2p.debian@free.fr> wrote:
> Signed-off-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
> ---
> 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, passe the skb as if no rx_handler was called.
s/passe/process/
^ permalink raw reply
* RFC: dsa slave open handling
From: Peter Korsgaard @ 2011-03-07 16:46 UTC (permalink / raw)
To: davem, buytenh, netdev
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.
As an example, I have a device with a single network interface connected
to a mv88e6060 switch. This is nicely handled through dsa, but with this
setup, IP autoconfiguration doesn't work:
If you boot with ip=on (E.G. use any device) eth0 is first brought up
and then the dsa slave devices. DHCP/bootp requests are sent out on both
the dsa slaves and the master eth0 interface. This is not optimal as the
mv88e6060 uses the trailer tag format, so it ends up stripping the
trailing 4 bytes of the requests sent directly on eth0 (and then sends
out on port 0), which leads to invalid UDP checksum, but that's worse -
Once a reply has been received on one of the dsa ports, autoconfig
closes all other devices including eth0, which also brings down the dsa
ports so nfs mount fails.
If on the other hand you boot with ip=:::::<dsa port name>
(E.G. explicitly force autoconfig to use that port), then it fails when
it tries to open the device because eth0 isn't up:
static int dsa_slave_open(struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
struct net_device *master = p->parent->dst->master_netdev;
int err;
if (!(master->flags & IFF_UP))
return -ENETDOWN;
How should this preferably be handled? We could either automatically
bring up the master device whenever a dsa slave is brought up:
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 64ca2a6..dfc7558 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -63,8 +63,9 @@ static int dsa_slave_open(struct net_device *dev)
struct net_device *master = p->parent->dst->master_netdev;
int err;
- if (!(master->flags & IFF_UP))
- return -ENETDOWN;
+ err = dev_change_flags(master, master->flags | IFF_UP);
+ if (err < 0)
+ return err;
if (compare_ether_addr(dev->dev_addr, master->dev_addr)) {
err = dev_uc_add(master, dev->dev_addr);
dev_change_flags(master, master->flags | IFF_UP);
Or we could simply handle it in ipconfig by not closing dsa master devices:
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 2b09775..4d4474b 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -277,6 +277,11 @@ static void __init ic_close_devs(void)
next = d->next;
dev = d->dev;
if (dev != ic_dev) {
+#ifdef CONFIG_NET_DSA
+ /* master device might be needed for dsa access */
+ if (dev->dsa_ptr)
+ continue;
+#endif
DBG(("IP-Config: Downing %s\n", dev->name));
dev_change_flags(dev, d->flags);
}
Which is preferred?
--
Bye, Peter Korsgaard
^ permalink raw reply related
* [PATCH] vxge: update MAINTAINERS
From: Jon Mason @ 2011-03-07 17:02 UTC (permalink / raw)
To: davem; +Cc: netdev, Jon Mason
Exar has exited the 10G Ethernet NIC market, orphaning both the s2io and
vxge drivers. With the promise of free hardware, I'll take over
maintainership.
Signed-off-by: Jon Mason <jdmason@kudzu.us>
---
MAINTAINERS | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 75760e7..c069c45 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4284,10 +4284,7 @@ S: Maintained
F: net/sched/sch_netem.c
NETERION 10GbE DRIVERS (s2io/vxge)
-M: Ramkrishna Vepa <ramkrishna.vepa@exar.com>
-M: Sivakumar Subramani <sivakumar.subramani@exar.com>
-M: Sreenivasa Honnur <sreenivasa.honnur@exar.com>
-M: Jon Mason <jon.mason@exar.com>
+M: Jon Mason <jdmason@kudzu.us>
L: netdev@vger.kernel.org
W: http://trac.neterion.com/cgi-bin/trac.cgi/wiki/Linux?Anonymous
W: http://trac.neterion.com/cgi-bin/trac.cgi/wiki/X3100Linux?Anonymous
--
1.7.0.4
^ permalink raw reply related
* Re: [ethtool PATCH 2/2] Add RX packet classification interface
From: Alexander Duyck @ 2011-03-07 17:04 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <1299513430.2522.9.camel@bwh-desktop>
On 3/7/2011 7:57 AM, Ben Hutchings wrote:
> On Fri, 2011-03-04 at 11:09 -0800, Alexander Duyck wrote:
>> On 2/28/2011 4:35 PM, Ben Hutchings wrote:
>>> On Tue, 2011-02-22 at 12:52 -0800, Alexander Duyck wrote:
>>
>> [...]
>>
>>>>>> } else
>>>>>> show_usage(1);
>>>>>> break;
>>>>>
>>>>> I don't think the same options (-n, -N) should be used both for flow
>>>>> hashing and n-tuple flow steering/filtering. This command-line
>>>>> interface and the structure used in the ethtool API just seem to reflect
>>>>> the implementation in the niu driver.
>>>>>
>>>>> (In fact I would much prefer it if the -u and -U options could be used
>>>>> for both the rxnfc and rxntuple interfaces. But I haven't thought about
>>>>> how the differences in functionality would be exposed to or hidden from
>>>>> the user.)
>>>>
>>>> I was kind of thinking about merging the two interfaces too, but I was
>>>> looking at it more from the perspective of moving away from ntuple more
>>>> towards this newer interface. My main motivation being that the filter
>>>> display option is so badly broken for ntuple that it would be easier to
>>>> make ntuple a subset of the flow classifier instead of the other way around.
>>>>
>>>> What would you think of using the "flow-type" keyword to indicate legacy
>>>> ntuple support, and then adding something like "class-rule-add", and
>>>> "class-rule-del" to add support for the network flow classifier calls?
>>>
>>> I really don't want to introduce different syntax for functionality that
>>> is common between the two command sets. The user should not have to
>>> know that driver A implements interface I and driver B implements
>>> interface J, except that since version 2.6.y driver A implements
>>> interface J too.
>>>
>>> Surely it is possible to try one interface, then the other, when the
>>> requested filter can be implemented either way?
>>
>> The problem is that the interfaces are different in the way they
>> implement their masks. N-tuple defines the mask as 0s mean inclusion,
>> 1s, mean exclusion.
>
> You have got to be kidding me!
>
> If this is the case, then the current kernel-doc for
> ethtool_rx_flow_spec::m_u is incorrect.
That would be the case. The m_u for ethtool_rx_flow_spec is 0 for bits
to be ignored. It is one of the things I really liked about that since
in combination with the way the original patch generated the masks it
would mean no goofy bit setting workarounds.
I think the documentation was added after the ethtool_rx_flow_spec and
ethtool_rx_ntuple_flow_spec were and it looks like whoever added it
probably assumed it worked the same way as ntuple. I can probably
submit an updated patch to correct the kernel-doc for that.
>> The network flow classifier filters are the exact
>> opposite. As such we really need to know which type of filter we are
>> dealing with before we start setting up values.
>
> This is nonsense; it's not hard to flip the bits later.
Sorry about that I was probably thinking a bit too literally. So what
you are suggesting is that I store the values as a 1's compliment of
what the user set as input for ethtool_rx_flow_spec. I think I can
manage that.
>> In addition there are
>> options such as location which exist in network flow classifier rules,
>> but not in ntuple rules.
>
> Sure, and when those are specified then there can be no fallback.
>
> Ben.
>
Actually now that I am thinking about it I could probably just ignore
location for rules that end up being processed via ntuple.
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.
The idea I have for this now is to just record everything using the
ethtool_rx_flow_spec. With it extended to support VLAN and 64 bytes of
data we should be able to just store everything in the one structure,
try recording it to the hardware via the nfc interface, if that fails
then copy the values except for location into a ntuple structure, and
attempt to write it via the ntuple interface.
Thanks,
Alex
^ permalink raw reply
* Re: [ethtool PATCH 2/2] Add RX packet classification interface
From: Ben Hutchings @ 2011-03-07 17:57 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <4D751038.9040804@intel.com>
On Mon, 2011-03-07 at 09:04 -0800, Alexander Duyck wrote:
> On 3/7/2011 7:57 AM, Ben Hutchings wrote:
> > On Fri, 2011-03-04 at 11:09 -0800, Alexander Duyck wrote:
> >> On 2/28/2011 4:35 PM, Ben Hutchings wrote:
> >>> On Tue, 2011-02-22 at 12:52 -0800, Alexander Duyck wrote:
> >>
> >> [...]
> >>
> >>>>>> } else
> >>>>>> show_usage(1);
> >>>>>> break;
> >>>>>
> >>>>> I don't think the same options (-n, -N) should be used both for flow
> >>>>> hashing and n-tuple flow steering/filtering. This command-line
> >>>>> interface and the structure used in the ethtool API just seem to reflect
> >>>>> the implementation in the niu driver.
> >>>>>
> >>>>> (In fact I would much prefer it if the -u and -U options could be used
> >>>>> for both the rxnfc and rxntuple interfaces. But I haven't thought about
> >>>>> how the differences in functionality would be exposed to or hidden from
> >>>>> the user.)
> >>>>
> >>>> I was kind of thinking about merging the two interfaces too, but I was
> >>>> looking at it more from the perspective of moving away from ntuple more
> >>>> towards this newer interface. My main motivation being that the filter
> >>>> display option is so badly broken for ntuple that it would be easier to
> >>>> make ntuple a subset of the flow classifier instead of the other way around.
> >>>>
> >>>> What would you think of using the "flow-type" keyword to indicate legacy
> >>>> ntuple support, and then adding something like "class-rule-add", and
> >>>> "class-rule-del" to add support for the network flow classifier calls?
> >>>
> >>> I really don't want to introduce different syntax for functionality that
> >>> is common between the two command sets. The user should not have to
> >>> know that driver A implements interface I and driver B implements
> >>> interface J, except that since version 2.6.y driver A implements
> >>> interface J too.
> >>>
> >>> Surely it is possible to try one interface, then the other, when the
> >>> requested filter can be implemented either way?
> >>
> >> The problem is that the interfaces are different in the way they
> >> implement their masks. N-tuple defines the mask as 0s mean inclusion,
> >> 1s, mean exclusion.
> >
> > You have got to be kidding me!
> >
> > If this is the case, then the current kernel-doc for
> > ethtool_rx_flow_spec::m_u is incorrect.
>
> That would be the case. The m_u for ethtool_rx_flow_spec is 0 for bits
> to be ignored. It is one of the things I really liked about that since
> in combination with the way the original patch generated the masks it
> would mean no goofy bit setting workarounds.
>
> I think the documentation was added after the ethtool_rx_flow_spec and
> ethtool_rx_ntuple_flow_spec were and it looks like whoever added it
> probably assumed it worked the same way as ntuple. I can probably
> submit an updated patch to correct the kernel-doc for that.
I added that documentation. Since I missed the original rxnfc patch for
ethtool, from which I could have inferred the correct semantics of the
masks, I assumed that they were interrpeted the same as in
ethtool_rx_ntuple_flow_spec.
[...]
> Actually now that I am thinking about it I could probably just ignore
> location for rules that end up being processed via ntuple.
>
> 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. 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'.
> The idea I have for this now is to just record everything using the
> ethtool_rx_flow_spec. With it extended to support VLAN and 64 bytes of
> data we should be able to just store everything in the one structure,
> try recording it to the hardware via the nfc interface, if that fails
> then copy the values except for location into a ntuple structure, and
> attempt to write it via the ntuple interface.
Right.
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 V4] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Alexander Beregalov @ 2011-03-07 18:04 UTC (permalink / raw)
To: Jesse Barnes
Cc: Narendra_K, linux-pci, linux-hotplug, netdev, mjg, Matt_Domsch,
Charles_Rose, Jordan_Hargrave, Shyam_Iyer, sfr
In-Reply-To: <20110304104314.26265021@jbarnes-desktop>
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'
^ permalink raw reply
* Re: [ethtool PATCH 2/2] Add RX packet classification interface
From: Dimitris Michailidis @ 2011-03-07 18:22 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Alexander Duyck, Santwona Behera, netdev@vger.kernel.org
In-Reply-To: <1299520664.2522.21.camel@bwh-desktop>
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.
> 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.
^ 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
* 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: [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
* [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: 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
* 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: 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: [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
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