* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: John Fastabend @ 2013-02-26 21:58 UTC (permalink / raw)
To: vyasevic; +Cc: David Miller, netdev
In-Reply-To: <512D1A01.9020301@redhat.com>
[...]
>>>
>>>
>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw
>>> addresses.
>>>
>>> Add an ability to add and remove HW addresses to the device
>>> unicast and multicast address lists. Right now, we only have
>>> an ioctl() to manage the multicast addresses and there is no
>>> way the manage the unicast list.
>>>
>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>
>> This is a step in the right direction, and you're right that there is
>> a difficulty in detecting whether support exists or not.
>>
>> I am so surprised that we've have ->set_rx_mode() support for multiple
>> unicast MAC addresses in so many drivers all this time, yet no way
>> outside of FDB to make use of it at all.
>
> And even that is not always available. In most drivers it requires
> module parameters or other explicit configuration steps. Meanwhile
> set_rx_mode() doesn't seem to depend on any of those and just does the
> right thing.
>
> For what I was trying to do ioctl() was a really easy way out for both
> kernel and user space implementation, so I gave is shot.
>
> -vlad
>
Don't we already support this with
int (*ndo_fdb_add)(struct ndmsg *ndm,
struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr,
u16 flags);
int (*ndo_fdb_del)(struct ndmsg *ndm,
struct nlattr *tb[],
struct net_device *dev,
const unsigned char *addr);
int (*ndo_fdb_dump)(struct sk_buff *skb,
struct
netlink_callback *cb,
struct net_device *dev,
int idx);
^ permalink raw reply
* LSM stacking and the network access controls (was: AF_VSOCK and the LSMs)
From: Paul Moore @ 2013-02-26 21:21 UTC (permalink / raw)
To: Casey Schaufler
Cc: netdev, linux-security-module, selinux, Andy King, Gerd Hoffmann,
Eric Paris
In-Reply-To: <512BEE66.9030405@schaufler-ca.com>
On Monday, February 25, 2013 03:06:14 PM Casey Schaufler wrote:
> On 2/25/2013 1:05 PM, Paul Moore wrote:
> > On Monday, February 25, 2013 10:02:42 AM Casey Schaufler wrote:
> >> On 2/25/2013 8:55 AM, Paul Moore wrote:
> >>> [NOTE/WARNING: we're veering away from the VSOCK discussion and towards
> >>> a LSM stacking discussion; see my response to Gerd if you want to stay
> >>> on topic.]
> >>>
> >>> On Saturday, February 23, 2013 03:43:23 PM Casey Schaufler wrote:
> >>>> On 2/22/2013 4:45 PM, Paul Moore wrote:
> >>>>> On Friday, February 22, 2013 03:00:04 PM Casey Schaufler wrote:
> >>>>>> Please add an LSM blob. Please do not use a secid. I am currently
> >>>>>> battling with secids in my efforts for multiple LSM support.
> >>>>>>
> >>>>>> ...
> >>>>>>
> >>>>>> I am going to be able to deal with secids for AF_INET only because
> >>>>>> SELinux prefers XFRM, Smack requires CIPSO, and AppArmor is going to
> >>>>>> be willing to have networking be optional.
> >>>>>
> >>>>> "prefers"? Really Casey, did you think I would let you get away with
> >>>>> that statement? What a LSM "prefers" is really not relevant to the
> >>>>> stacking effort, what a LSM _supports_ is what matters.
> >>>>
> >>>> I suppose. My point, which you may refute if it is incorrect,
> >>>> is that there are common, legitimate SELinux configurations which
> >>>> eschew Netlabel in favor of XFRM.
> >>>
> >>> There are common, legitimate use cases which use exclusively NetLabel,
> >>> exclusively labeled IPsec, and both. A LSM stacking design that forces
> >>> SELinux to only operate with XFRM (labeled IPsec) is wrong. If you are
> >>> giving admins the ability to selectively stack LSMs you should also
> >>> allow them to selectively pick which non-shareable functionality they
> >>> assign to each LSM.
> >>
> >> That is the approach I'm taking. The kernel configuration
> >> will specify which LSM gets Netlabel and which gets XFRM.
> >
> > Okay, it wasn't obvious to me in your previous emails that this was the
> > case. Also, just so I'm clear, you configure the stacking and the stacked
> > network access controls at the same time, yes? I want to avoid the
> > situation where the stacked network access controls are determined at
> > build time and the LSM stacking order/configuration is determined at
> > boot.
>
> The set of LSMs, the order they are invoked, which LSM
> uses /proc/.../attr/current and which LSM uses Netlabel,
> XFRM and secmark are all determined by Kconfig. You can
> specify a limited set of LSMs using security= at boot,
> but not the networking configuration.
That's unfortunate. I'm _really_ not in favor of that, I would much rather
see the non-shared LSM functionality assigned at the same time as the stacking
order. I'm not sure I'd NACK the current approach, or even if anyone would
care that I did, but that is how I'm currently leaning with this split (build
vs runtime) selection.
> Tetsuo is lobbying for loadable security modules. I am
> doing my best to leave everything in a state where some
> soul braver than I might pick that up after I'm done.
> I do not have any intention of supporting on the fly
> or heuristically determined networking.
Well, if that is the case I think the first-come-first-served approach to the
non-shared functionality probably makes the most sense. I understand why it
might not be ideal in ever situation but it is predictable.
> >> I'm trying not to make too many architectural changes to the
> >> code around the LSM mechanism itself. I don't see that as cost
> >> effective or likely to be popular. If the implication of that is
> >> that there are certain configurations that are unsupportable but
> >> that have plausible alternatives I think it will do for phase I.
> >
> > That statement is vague enough that I can't really say yes or no to it,
> > but I will stick by my previous comments about the network access
> > controls.
>
> Ah. I want to create a useful system. I want to create it
> reasonably short order. I am willing to forgo supporting
> some configuration options to reduce the project time. In
> particular, I hope to avoid mucking up other people's code
> as that is a sure fire way to bog a project down.
Usefulness is paramount of course and quickness is nice, but I do think it
takes a back seat to "the right solution". We're all going to have to live
with this f-o-r-e-v-e-r, or at least what will surely seem like it, so I'd
much rather we take the time to make sure we beat out as much ugly as we can
before it is merged.
> >>>> Options I have considered include:
> >>>> - Netlabel support for discriminating LSM use by host,
> >>>>
> >>>> just as it currently allows for unlabeled hosts.
> >>>
> >>> Hmm, interesting ... not sure what I think of this.
> >>
> >> It breaks down on 127.0.0.1. Otherwise is looks pretty easy to do.
> >
> > When I said I wasn't sure what I thought of this, it was more along the
> > lines of I'm not sure if it makes sense, not that it would be difficult
> > to do.
>
> It makes sense if you're in a network environment with heterogeneous
> legacy multilevel secure systems and you want some of those machines
> to talk to SELinux controlled processes and others to talk to Smack
> controlled processes. That is unlikely to be an industry dominating
> scenario.
Well, you can talk MLS labeled networking to both SELinux and Smack, and you
can talk MLS labeled networking between SELinux and Smack already (hip, hip,
hooray for commonly accepted standards!) so I'm not sure you need this.
Although I suppose it might simplify the configuration in some cases or help
if you're trying to migrate from one LSM to another.
--
paul moore
security and virtualization @ redhat
^ permalink raw reply
* Re: [PATCH] usb/net/asix_devices: Add USBNET HG20F9 ethernet dongle
From: Richard Ash @ 2013-02-26 21:09 UTC (permalink / raw)
To: David Miller; +Cc: gregkh, linux-usb, netdev, linux-kernel
In-Reply-To: <20130226.001022.1341620780921182778.davem@davemloft.net>
On Tue, 26 Feb 2013 00:10:22 -0500 (EST)
David Miller <davem@davemloft.net> wrote:
> It didn't make it to any of the lists, that's why you are the
> only person who saw the original patch.
I've got it (the original send) here, so it seems to have gone through
to linux-usb@vger.kernel.org (or at least some of it) without problems.
I hope gmail has hidden the re-send from me - otherwise I didn't get
it ...
Richard
^ permalink raw reply
* qeth_l2 duplicated frames in promiscuous mode
From: Benjamin Poirier @ 2013-02-26 21:08 UTC (permalink / raw)
To: Ursula Braun, Frank Blaschka, Martin Schwidefsky, Heiko Carstens
Cc: Mark Post, linux390, linux-s390, netdev
Hello,
While working with the qeth/qeth_l2 drivers I've noticed that once the
nic enters promiscuous mode, every received frame is duplicated. This
appears to happen at the (virtual) hardware level since the "RX Packets"
counter printed by `#cp q nic [...] det` shows this duplicated count. It
also happens when setting promiscuous mode using `#cp set nic [...]
pro`.
It can be witnessed in tcpdump output or simply by pinging the machine
from another host. As soon as promiscuous mode is enabled using the cp
command above, the machine receives the echo request in duplicate and
answers them accordingly:
64 bytes from 10.121.155.133: icmp_seq=61 ttl=64 time=0.314 ms
64 bytes from 10.121.155.133: icmp_seq=62 ttl=64 time=0.308 ms
# enable promiscuous mode on 10.121.155.133 now
64 bytes from 10.121.155.133: icmp_seq=63 ttl=64 time=0.301 ms
64 bytes from 10.121.155.133: icmp_seq=63 ttl=64 time=0.326 ms (DUP!)
64 bytes from 10.121.155.133: icmp_seq=64 ttl=64 time=0.309 ms
64 bytes from 10.121.155.133: icmp_seq=64 ttl=64 time=0.330 ms (DUP!)
This duplication happens for multicast and unicast frames, whether those
are directed to an address in the uc filter list or not.
Tested on v3.8.
Is this a known issue?
Here is the nic info, let me know if you need anything else, I'm not
familiar with s390.
CP Q NIC 0800 DET
Adapter 0800.P00 Type: QDIO Name: UNASSIGNED Devices: 3
MAC: 02-00-00-00-00-07 VSWITCH: SYSTEM VSWNL2
RX Packets: 3005625 Discarded: 1318 Errors: 0
TX Packets: 287690 Discarded: 0 Errors: 0
RX Bytes: 3777624734 TX Bytes: 32040428
Connection Name: HALLOLE State: Session Established
Device: 0800 Unit: 000 Role: CTL-READ
Device: 0801 Unit: 001 Role: CTL-WRITE
Device: 0802 Unit: 002 Role: DATA vPort: 0264 Index: 0264
VLAN: 0556
Options: Ethernet Broadcast Promiscuous
Unicast MAC Addresses:
02-00-00-00-00-07
Multicast MAC Addresses:
01-00-5E-00-00-01
33-33-00-00-00-01
33-33-00-00-02-02
33-33-FF-00-00-07
Thank you,
-Benjamin
^ permalink raw reply
* RE: [PATCH] libertas sdio: remove CMD_FUNC_INIT call
From: Bing Zhao @ 2013-02-26 20:59 UTC (permalink / raw)
To: Daniel Drake
Cc: Lubomir Rintel, Harro Haan, Dan Williams,
libertas-dev@lists.infradead.org, netdev@vger.kernel.org,
linux-wireless@vger.kernel.org, John W. Linville,
linux-kernel@vger.kernel.org
In-Reply-To: <CAMLZHHTnRUT0pbpKR7=wVSDkOxo65tnA_JWYGtq9rejsVy2scQ@mail.gmail.com>
Hi Daniel,
> > Do you have any concern for OLPC platforms with above change? If it doesn't seem to break OLPC I
> will send a patch to the list.
>
> Looks fine to me. Thanks for investigating.
Thanks. I've just sent the patch.
Regards,
Bing
^ permalink raw reply
* [PATCH 3.9] libertas: fix crash for SD8688
From: Bing Zhao @ 2013-02-26 20:58 UTC (permalink / raw)
To: Lubomir Rintel, Dan Williams, Harro Haan
Cc: libertas-dev, netdev, linux-wireless, John W. Linville,
linux-kernel, Daniel Drake, Bing Zhao
For SD8688, FUNC_INIT command is queued before fw_ready flag is
set. This causes the following crash as lbs_thread blocks any
command if fw_ready is not set.
[ 209.338953] [<c0502248>] (__schedule+0x610/0x764) from [<bf20ae24>] (__lbs_cmd+0xb8/0x130 [libertas])
[ 209.348340] [<bf20ae24>] (__lbs_cmd+0xb8/0x130 [libertas]) from [<bf222474>] (if_sdio_finish_power_on+0xec/0x1b0 [libertas_sdio])
[ 209.360136] [<bf222474>] (if_sdio_finish_power_on+0xec/0x1b0 [libertas_sdio]) from [<bf2226c4>] (if_sdio_power_on+0x18c/0x20c [libertas_sdio])
[ 209.373052] [<bf2226c4>] (if_sdio_power_on+0x18c/0x20c [libertas_sdio]) from [<bf222944>] (if_sdio_probe+0x200/0x31c [libertas_sdio])
[ 209.385316] [<bf222944>] (if_sdio_probe+0x200/0x31c [libertas_sdio]) from [<bf01d820>] (sdio_bus_probe+0x94/0xfc [mmc_core])
[ 209.396748] [<bf01d820>] (sdio_bus_probe+0x94/0xfc [mmc_core]) from [<c02e729c>] (driver_probe_device+0x12c/0x348)
[ 209.407214] [<c02e729c>] (driver_probe_device+0x12c/0x348) from [<c02e7530>] (__driver_attach+0x78/0x9c)
[ 209.416798] [<c02e7530>] (__driver_attach+0x78/0x9c) from [<c02e5658>] (bus_for_each_dev+0x50/0x88)
[ 209.425946] [<c02e5658>] (bus_for_each_dev+0x50/0x88) from [<c02e6810>] (bus_add_driver+0x108/0x268)
[ 209.435180] [<c02e6810>] (bus_add_driver+0x108/0x268) from [<c02e782c>] (driver_register+0xa4/0x134)
[ 209.444426] [<c02e782c>] (driver_register+0xa4/0x134) from [<bf22601c>] (if_sdio_init_module+0x1c/0x3c [libertas_sdio])
[ 209.455339] [<bf22601c>] (if_sdio_init_module+0x1c/0x3c [libertas_sdio]) from [<c00085b8>] (do_one_initcall+0x98/0x174)
[ 209.466236] [<c00085b8>] (do_one_initcall+0x98/0x174) from [<c0076504>] (load_module+0x1c5c/0x1f80)
[ 209.475390] [<c0076504>] (load_module+0x1c5c/0x1f80) from [<c007692c>] (sys_init_module+0x104/0x128)
[ 209.484632] [<c007692c>] (sys_init_module+0x104/0x128) from [<c0008c40>] (ret_fast_syscall+0x0/0x38)
Fix it by setting fw_ready flag prior to queuing FUNC_INIT command.
Cc: <stable@vger.kernel.org> # 3.5+
Reported-by: Lubomir Rintel <lkundrak@v3.sk>
Tested-by: Lubomir Rintel <lkundrak@v3.sk>
Signed-off-by: Bing Zhao <bzhao@marvell.com>
---
drivers/net/wireless/libertas/if_sdio.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 739309e..4557833 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -825,6 +825,11 @@ static void if_sdio_finish_power_on(struct if_sdio_card *card)
sdio_release_host(func);
+ /* Set fw_ready before queuing any commands so that
+ * lbs_thread won't block from sending them to firmware.
+ */
+ priv->fw_ready = 1;
+
/*
* FUNC_INIT is required for SD8688 WLAN/BT multiple functions
*/
@@ -839,7 +844,6 @@ static void if_sdio_finish_power_on(struct if_sdio_card *card)
netdev_alert(priv->dev, "CMD_FUNC_INIT cmd failed\n");
}
- priv->fw_ready = 1;
wake_up(&card->pwron_waitq);
if (!card->started) {
--
1.7.0.2
^ permalink raw reply related
* Re: 3.7.8/amd64 full interrupt hangs due to iwlwifi under big nfs copies out
From: Johannes Berg @ 2013-02-26 20:54 UTC (permalink / raw)
To: Marc MERLIN
Cc: Eric Dumazet, David Miller, Larry.Finger, bhutchings,
linux-wireless, netdev
In-Reply-To: <20130223061414.GA16883@merlins.org>
On Fri, 2013-02-22 at 22:14 -0800, Marc MERLIN wrote:
> On Wed, Feb 20, 2013 at 10:15:03AM +0100, Johannes Berg wrote:
> > On Wed, 2013-02-20 at 10:12 +0100, Johannes Berg wrote:
> > > On Tue, 2013-02-19 at 08:21 -0800, Eric Dumazet wrote:
> > > > On Tue, 2013-02-19 at 11:03 +0100, Johannes Berg wrote:
> > > > > On Mon, 2013-02-18 at 21:17 -0800, Eric Dumazet wrote:
> > > > >
> > > > > > > chrome: page allocation failure: order:1, mode:0x4020
> > > > > > > Pid: 8730, comm: chrome Tainted: G O 3.7.8-amd64-preempt-20121226-fixwd #1
> > > > > > > Call Trace:
> > > > > > > <IRQ> [<ffffffff810d5f38>] warn_alloc_failed+0x117/0x12c
> > > > >
> > > > > > You could try to load iwlwifi with amsdu_size_8K set to 0 (disable)
> > > > > >
> > > > > > It should hopefully use order-0 pages
> > > > >
> > > > > It will, do that then, unfortunately it can't switch at runtime because
> > > > > it advertised this support to the access point or clients.
> > > >
> > > > What are the drawbacks of setting amsdu_size_8K to 0 by default ?
> > >
> > > We're discussing this now, the only downside would be that we couldn't
> > > receive 8k A-MSDUs. Thing is, practically nobody uses A-MSDU anyway, and
> > > even when I suspect the difference between 4k and 8k won't be huge.
> >
> > OTOH, this affects the protocol, and when you really can't allocate any
> > order-1 pages you pointed out yourself that many other things also won't
> > work, so I'm not really sure it makes a big difference if we change the
> > driver?
>
> That as an unscientific test, but when I did the NFS eats all my pages
> test using ethernet, my system didn't hang like it did with iwlagn.
>
> So while the NFS code is definitely doing something wrong when it uses
> its default huge buffers, the e1000e code deals with it without hanging
> my system.
>
> So thanks for trying to improve the iwlagn code to avoid those system
> lockups.
We'll be submitting a patch to make single pages default.
johannes
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: David Miller @ 2013-02-26 20:48 UTC (permalink / raw)
To: vyasevic; +Cc: netdev
In-Reply-To: <512D1A01.9020301@redhat.com>
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 26 Feb 2013 15:24:33 -0500
> For what I was trying to do ioctl() was a really easy way out for both
> kernel and user space implementation, so I gave is shot.
Look at it this way, you were going to have to add these config values
to the rtnetlink LINK dumps anyways. :-)
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: Vlad Yasevich @ 2013-02-26 20:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20130226.150309.1215210522427869530.davem@davemloft.net>
On 02/26/2013 03:03 PM, David Miller wrote:
> From: Vlad Yasevich <vyasevic@redhat.com>
> Date: Tue, 26 Feb 2013 14:44:22 -0500
>
>> David Miller wrote:
>>> No new ioctls please, extend the netlink interfaces as necessary
>>> instead.
>>>
>>> Thanks.
>>
>> So, something like below patch (compiled only) would be better in your opinion?
>> The one concern I have is that there is no way to probe for the availabilty
>> of this functionality, since netlink will just ignore unknown types and appear
>> to succeed. With an ioctl it is clear if the support is present.
>>
>> -- >8 --
>>
>>
>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw addresses.
>>
>> Add an ability to add and remove HW addresses to the device
>> unicast and multicast address lists. Right now, we only have
>> an ioctl() to manage the multicast addresses and there is no
>> way the manage the unicast list.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>
> This is a step in the right direction, and you're right that there is
> a difficulty in detecting whether support exists or not.
>
> I am so surprised that we've have ->set_rx_mode() support for multiple
> unicast MAC addresses in so many drivers all this time, yet no way
> outside of FDB to make use of it at all.
And even that is not always available. In most drivers it requires
module parameters or other explicit configuration steps. Meanwhile
set_rx_mode() doesn't seem to depend on any of those and just does the
right thing.
For what I was trying to do ioctl() was a really easy way out for both
kernel and user space implementation, so I gave is shot.
-vlad
>
> Anyways, as per the feature detection issue, we could create a new
> RTM_* operation that returns a u32 feature flag mask. It's just one
> idea.
>
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: David Miller @ 2013-02-26 20:03 UTC (permalink / raw)
To: vyasevic; +Cc: netdev
In-Reply-To: <1361907862-13009-1-git-send-email-vyasevic@redhat.com>
From: Vlad Yasevich <vyasevic@redhat.com>
Date: Tue, 26 Feb 2013 14:44:22 -0500
> David Miller wrote:
>> No new ioctls please, extend the netlink interfaces as necessary
>> instead.
>>
>> Thanks.
>
> So, something like below patch (compiled only) would be better in your opinion?
> The one concern I have is that there is no way to probe for the availabilty
> of this functionality, since netlink will just ignore unknown types and appear
> to succeed. With an ioctl it is clear if the support is present.
>
> -- >8 --
>
>
> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw addresses.
>
> Add an ability to add and remove HW addresses to the device
> unicast and multicast address lists. Right now, we only have
> an ioctl() to manage the multicast addresses and there is no
> way the manage the unicast list.
>
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
This is a step in the right direction, and you're right that there is
a difficulty in detecting whether support exists or not.
I am so surprised that we've have ->set_rx_mode() support for multiple
unicast MAC addresses in so many drivers all this time, yet no way
outside of FDB to make use of it at all.
Anyways, as per the feature detection issue, we could create a new
RTM_* operation that returns a u32 feature flag mask. It's just one
idea.
^ permalink raw reply
* [PATCH] xilinx: ll_temac: Remove redundant NULL check before kfree
From: Syam Sidhardhan @ 2013-02-26 19:59 UTC (permalink / raw)
To: netdev; +Cc: syamsidhardh, John.Linn, anirudh, davem
kfree on NULL pointer is a no-op.
Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 9fc2ada..200a341 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -224,8 +224,7 @@ static void temac_dma_bd_release(struct net_device *ndev)
dma_free_coherent(ndev->dev.parent,
sizeof(*lp->tx_bd_v) * TX_BD_NUM,
lp->tx_bd_v, lp->tx_bd_p);
- if (lp->rx_skb)
- kfree(lp->rx_skb);
+ kfree(lp->rx_skb);
}
/**
--
1.7.9.5
^ permalink raw reply related
* [PATCH] qlcnic: Remove redundant NULL check before kfree
From: Syam Sidhardhan @ 2013-02-26 19:55 UTC (permalink / raw)
To: netdev, linux-driver; +Cc: syamsidhardh, sony.chacko, jitendra.kalsaria
kfree on NULL pointer is a no-op.
Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 28a6d48..b4c33c2 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -192,8 +192,7 @@ int qlcnic_alloc_sds_rings(struct qlcnic_recv_context *recv_ctx, int count)
void qlcnic_free_sds_rings(struct qlcnic_recv_context *recv_ctx)
{
- if (recv_ctx->sds_rings != NULL)
- kfree(recv_ctx->sds_rings);
+ kfree(recv_ctx->sds_rings);
recv_ctx->sds_rings = NULL;
}
@@ -2038,10 +2037,8 @@ static void qlcnic_remove(struct pci_dev *pdev)
qlcnic_detach(adapter);
- if (adapter->npars != NULL)
- kfree(adapter->npars);
- if (adapter->eswitch != NULL)
- kfree(adapter->eswitch);
+ kfree(adapter->npars);
+ kfree(adapter->eswitch);
if (qlcnic_82xx_check(adapter))
qlcnic_clr_all_drv_state(adapter, 0);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] netxen: Remove redundant NULL check before kfree
From: Syam Sidhardhan @ 2013-02-26 19:53 UTC (permalink / raw)
To: netdev; +Cc: syamsidhardh, sony.chacko, rajesh.borundia
kfree on NULL pointer is a no-op.
Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
---
.../net/ethernet/qlogic/netxen/netxen_nic_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 501f492..377b521 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -178,8 +178,7 @@ netxen_alloc_sds_rings(struct netxen_recv_context *recv_ctx, int count)
static void
netxen_free_sds_rings(struct netxen_recv_context *recv_ctx)
{
- if (recv_ctx->sds_rings != NULL)
- kfree(recv_ctx->sds_rings);
+ kfree(recv_ctx->sds_rings);
recv_ctx->sds_rings = NULL;
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH] myri10ge: Remove redundant NULL check before kfree
From: Syam Sidhardhan @ 2013-02-26 19:51 UTC (permalink / raw)
To: netdev; +Cc: syamsidhardh, gallatin
kfree on NULL pointer is a no-op.
Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
---
drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 4f9937e..2f9efe8 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -4005,8 +4005,7 @@ static void myri10ge_remove(struct pci_dev *pdev)
mtrr_del(mgp->mtrr, mgp->iomem_base, mgp->board_span);
#endif
myri10ge_free_slices(mgp);
- if (mgp->msix_vectors != NULL)
- kfree(mgp->msix_vectors);
+ kfree(mgp->msix_vectors);
dma_free_coherent(&pdev->dev, sizeof(*mgp->cmd),
mgp->cmd, mgp->cmd_bus);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] cirrus: Remove redundant NULL check before kfree
From: Syam Sidhardhan @ 2013-02-26 19:49 UTC (permalink / raw)
To: netdev; +Cc: syamsidhardh, hsweeten
kfree on NULL pointer is a no-op.
Signed-off-by: Syam Sidhardhan <s.syam@samsung.com>
---
drivers/net/ethernet/cirrus/ep93xx_eth.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cirrus/ep93xx_eth.c b/drivers/net/ethernet/cirrus/ep93xx_eth.c
index 354cbb7..298aaa9 100644
--- a/drivers/net/ethernet/cirrus/ep93xx_eth.c
+++ b/drivers/net/ethernet/cirrus/ep93xx_eth.c
@@ -476,8 +476,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
if (d)
dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_FROM_DEVICE);
- if (ep->rx_buf[i] != NULL)
- kfree(ep->rx_buf[i]);
+ kfree(ep->rx_buf[i]);
}
for (i = 0; i < TX_QUEUE_ENTRIES; i++) {
@@ -487,8 +486,7 @@ static void ep93xx_free_buffers(struct ep93xx_priv *ep)
if (d)
dma_unmap_single(dev, d, PKT_BUF_SIZE, DMA_TO_DEVICE);
- if (ep->tx_buf[i] != NULL)
- kfree(ep->tx_buf[i]);
+ kfree(ep->tx_buf[i]);
}
dma_free_coherent(dev, sizeof(struct ep93xx_descs), ep->descs,
--
1.7.9.5
^ permalink raw reply related
* [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: Vlad Yasevich @ 2013-02-26 19:44 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <20130226.124322.28904104699526114.davem@davemloft.net>
David Miller wrote:
> No new ioctls please, extend the netlink interfaces as necessary
> instead.
>
> Thanks.
So, something like below patch (compiled only) would be better in your opinion?
The one concern I have is that there is no way to probe for the availabilty
of this functionality, since netlink will just ignore unknown types and appear
to succeed. With an ioctl it is clear if the support is present.
-- >8 --
[RFC PATCH] rtnetlink: Add support for adding/removing additional hw addresses.
Add an ability to add and remove HW addresses to the device
unicast and multicast address lists. Right now, we only have
an ioctl() to manage the multicast addresses and there is no
way the manage the unicast list.
Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
---
include/uapi/linux/if_link.h | 3 +
include/uapi/linux/rtnetlink.h | 1 +
net/core/rtnetlink.c | 84 +++++++++++++++++++++++++++++++++++++++-
3 files changed, 87 insertions(+), 1 deletions(-)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index c4edfe1..7933a1c 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -143,6 +143,9 @@ enum {
IFLA_NUM_TX_QUEUES,
IFLA_NUM_RX_QUEUES,
IFLA_CARRIER,
+ IFLA_HW_ADDR_ADD,
+ IFLA_HW_ADDR_DEL,
+ IFLA_HW_ADDR_LIST,
__IFLA_MAX
};
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 7a2144e..4d6f1c9 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -631,6 +631,7 @@ struct tcamsg {
/* New extended info filters for IFLA_EXT_MASK */
#define RTEXT_FILTER_VF (1 << 0)
#define RTEXT_FILTER_BRVLAN (1 << 1)
+#define RTEXT_FILTER_MACLIST (1 << 2)
/* End of information exported to user level */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d8aa20f..4df2ff1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -763,6 +763,20 @@ static size_t rtnl_port_size(const struct net_device *dev)
return port_self_size;
}
+static size_t rtnl_maclist_size(const struct net_device *dev,
+ u32 ext_filter_mask)
+{
+ size_t size = 0;
+
+ if (ext_filter_mask & RTEXT_FILTER_MACLIST) {
+ size = nla_total_size(sizeof(struct nlattr)) +
+ nla_total_size(ETH_ALEN) *
+ (netdev_uc_count(dev) + netdev_mc_count(dev));
+ }
+
+ return size;
+}
+
static noinline size_t if_nlmsg_size(const struct net_device *dev,
u32 ext_filter_mask)
{
@@ -790,6 +804,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
& RTEXT_FILTER_VF ? 4 : 0) /* IFLA_NUM_VF */
+ rtnl_vfinfo_size(dev, ext_filter_mask) /* IFLA_VFINFO_LIST */
+ rtnl_port_size(dev) /* IFLA_VF_PORTS + IFLA_PORT_SELF */
+ + rtnl_maclist_size(dev, ext_filter_mask) /* IFLA_MAC_LIST */
+ rtnl_link_get_size(dev) /* IFLA_LINKINFO */
+ rtnl_link_get_af_size(dev); /* IFLA_AF_SPEC */
}
@@ -870,6 +885,39 @@ static int rtnl_port_fill(struct sk_buff *skb, struct net_device *dev)
return 0;
}
+static int rtnl_hwaddr_fill(struct sk_buff *skb, struct net_device *dev,
+ u32 ext_filter_mask)
+{
+ struct netdev_hw_addr *ha;
+ struct nlattr *hw_list;
+
+ if (!(ext_filter_mask & RTEXT_FILTER_MACLIST))
+ return 0;
+
+ if ((netdev_uc_count(dev) + netdev_mc_count(dev)) == 0)
+ return 0;
+
+ hw_list = nla_nest_start(skb, IFLA_HW_ADDR_LIST);
+ if (!hw_list)
+ return -EMSGSIZE;
+
+ /* Unicasts first */
+ netdev_for_each_uc_addr(ha, dev) {
+ if (nla_put(skb, IFLA_ADDRESS, ETH_ALEN, ha->addr))
+ goto nla_put_fail;
+ }
+
+ /* Now multicasts */
+ netdev_for_each_mc_addr(ha, dev) {
+ if (nla_put(skb, IFLA_ADDRESS, ETH_ALEN, ha->addr))
+ goto nla_put_fail;
+ }
+
+nla_put_fail:
+ nla_nest_cancel(skb, hw_list);
+ return -EMSGSIZE;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask)
@@ -1044,6 +1092,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
}
}
+ if (rtnl_hwaddr_fill(skb, dev, ext_filter_mask))
+ goto nla_put_failure;
+
nla_nest_end(skb, af_spec);
return nlmsg_end(skb, nlh);
@@ -1128,6 +1179,8 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_PROMISCUITY] = { .type = NLA_U32 },
[IFLA_NUM_TX_QUEUES] = { .type = NLA_U32 },
[IFLA_NUM_RX_QUEUES] = { .type = NLA_U32 },
+ [IFLA_HW_ADDR_ADD] = { .type = NLA_BINARY, .len = ETH_ALEN },
+ [IFLA_HW_ADDR_DEL] = { .type = NLA_BINARY, .len = ETH_ALEN },
};
EXPORT_SYMBOL(ifla_policy);
@@ -1527,7 +1580,36 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
modified = 1;
}
}
- err = 0;
+
+ if (tb[IFLA_HW_ADDR_ADD]) {
+ const unsigned char *addr;
+
+ addr = nla_data(tb[IFLA_HW_ADDR_ADD]);
+
+ if (is_unicast_ether_addr(addr))
+ err = dev_uc_add_excl(dev, addr);
+ else if (is_multicast_ether_addr(addr))
+ err = dev_mc_add_excl(dev, addr);
+
+ if (err)
+ goto errout;
+ modified = 1;
+ }
+
+ if (tb[IFLA_HW_ADDR_DEL]) {
+ const unsigned char *addr;
+
+ addr = nla_data(tb[IFLA_HW_ADDR_DEL]);
+
+ if (is_unicast_ether_addr(addr))
+ err = dev_uc_del(dev, addr);
+ else if (is_multicast_ether_addr(addr))
+ err = dev_mc_del(dev, addr);
+
+ if (err)
+ goto errout;
+ modified = 1;
+ }
errout:
if (err < 0 && modified)
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Srivatsa S. Bhat @ 2013-02-26 19:30 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Michel Lespinasse, linux-doc, peterz, fweisbec, linux-kernel,
namhyung, mingo, linux-arch, linux, xiaoguangrong, wangyun,
paulmck, nikunj, linux-pm, rusty, rostedt, rjw, vincent.guittot,
tglx, linux-arm-kernel, netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CACvQF50UgiqPpDk3HCmV86zKSf6tPjqr0bZowfPEOJs0g0r0MQ@mail.gmail.com>
On 02/26/2013 09:55 PM, Lai Jiangshan wrote:
> On Tue, Feb 26, 2013 at 10:22 PM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>
>> Hi Lai,
>>
>> I'm really not convinced that piggy-backing on lglocks would help
>> us in any way. But still, let me try to address some of the points
>> you raised...
>>
>> On 02/26/2013 06:29 PM, Lai Jiangshan wrote:
>>> On Tue, Feb 26, 2013 at 5:02 PM, Srivatsa S. Bhat
>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>> On 02/26/2013 05:47 AM, Lai Jiangshan wrote:
>>>>> On Tue, Feb 26, 2013 at 3:26 AM, Srivatsa S. Bhat
>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>>> Hi Lai,
>>>>>>
>>>>>> On 02/25/2013 09:23 PM, Lai Jiangshan wrote:
>>>>>>> Hi, Srivatsa,
>>>>>>>
>>>>>>> The target of the whole patchset is nice for me.
>>>>>>
>>>>>> Cool! Thanks :-)
>>>>>>
>>>> [...]
>>>>
>>>> Unfortunately, I see quite a few issues with the code above. IIUC, the
>>>> writer and the reader both increment the same counters. So how will the
>>>> unlock() code in the reader path know when to unlock which of the locks?
>>>
>>> The same as your code, the reader(which nested in write C.S.) just dec
>>> the counters.
>>
>> And that works fine in my case because the writer and the reader update
>> _two_ _different_ counters.
>
> I can't find any magic in your code, they are the same counter.
>
> /*
> * It is desirable to allow the writer to acquire the percpu-rwlock
> * for read (if necessary), without deadlocking or getting complaints
> * from lockdep. To achieve that, just increment the reader_refcnt of
> * this CPU - that way, any attempt by the writer to acquire the
> * percpu-rwlock for read, will get treated as a case of nested percpu
> * reader, which is safe, from a locking perspective.
> */
> this_cpu_inc(pcpu_rwlock->rw_state->reader_refcnt);
>
Whoa! Hold on, were you really referring to _this_ increment when you said
that, in your patch you would increment the refcnt at the writer? Then I guess
there is a major disconnect in our conversations. (I had assumed that you were
referring to the update of writer_signal, and were just trying to have a single
refcnt instead of reader_refcnt and writer_signal).
So, please let me clarify things a bit here. Forget about the above increment
of reader_refcnt at the writer side. Its almost utterly insignificant for our
current discussion. We can simply replace it with a check as shown below, at
the reader side:
void percpu_read_lock_irqsafe()
{
if (current == active_writer)
return;
/* Rest of the code */
}
Now, assuming that, in your patch, you were trying to use the per-cpu refcnt
to allow the writer to safely take the reader path, you can simply get rid of
that percpu-refcnt, as demonstrated above.
So that would reduce your code to the following (after simplification):
lg_rwlock_local_read_lock()
{
if (current == active_writer)
return;
if (arch_spin_trylock(per-cpu-spinlock))
return;
read_lock(global-rwlock);
}
Now, let us assume that hotplug is not happening, meaning, nobody is running
the writer side code. Now let's see what happens at the reader side in your
patch. As I mentioned earlier, the readers are _very_ frequent and can be in
very hot paths. And they also happen to be nested quite often.
So, a non-nested reader acquires the per-cpu spinlock. Every subsequent nested
reader on that CPU has to acquire the global rwlock for read. Right there you
have 2 significant performance issues -
1. Acquiring the (spin) lock is costly
2. Acquiring the global rwlock causes cacheline bouncing, which hurts
performance.
And why do we care so much about performance here? Because, the existing
kernel does an efficient preempt_disable() here - which is an optimized
per-cpu counter increment. Replacing that with such heavy primitives on the
reader side can be very bad.
Now, how does my patchset tackle this? My scheme just requires an increment
of a per-cpu refcnt (reader_refcnt) and memory barrier. Which is acceptable
from a performance-perspective, because IMHO its not horrendously worse than
a preempt_disable().
>
>> If both of them update the same counter, there
>> will be a semantic clash - an increment of the counter can either mean that
>> a new writer became active, or it can also indicate a nested reader. A decrement
>> can also similarly have 2 meanings. And thus it will be difficult to decide
>> the right action to take, based on the value of the counter.
>>
>>>
>>>> (The counter-dropping-to-zero logic is not safe, since it can be updated
>>>> due to different reasons). And now that I look at it again, in the absence
>>>> of the writer, the reader is allowed to be recursive at the heavy cost of
>>>> taking the global rwlock for read, every 2nd time you nest (because the
>>>> spinlock is non-recursive).
>>>
>>> (I did not understand your comments of this part)
>>> nested reader is considered seldom.
>>
>> No, nested readers can be _quite_ frequent. Because, potentially all users
>> of preempt_disable() are readers - and its well-known how frequently we
>> nest preempt_disable(). As a simple example, any atomic reader who calls
>> smp_call_function() will become a nested reader, because smp_call_function()
>> itself is a reader. So reader nesting is expected to be quite frequent.
>>
>>> But if N(>=2) nested readers happen,
>>> the overhead is:
>>> 1 spin_try_lock() + 1 read_lock() + (N-1) __this_cpu_inc()
>>>
>>
>> In my patch, its just this_cpu_inc(). Note that these are _very_ hot paths.
>> So every bit of optimization that you can add is worthwhile.
>>
>> And your read_lock() is a _global_ lock - thus, it can lead to a lot of
>> cache-line bouncing. That's *exactly* why I have used per-cpu refcounts in
>> my synchronization scheme, to avoid taking the global rwlock as much as possible.
>>
>> Another important point to note is that, the overhead we are talking about
>> here, exists even when _not_ performing hotplug. And its the replacement to
>> the super-fast preempt_disable(). So its extremely important to consciously
>> minimize this overhead - else we'll end up slowing down the system significantly.
>>
>
> All I was considered is "nested reader is seldom", so I always
> fallback to rwlock when nested.
> If you like, I can add 6 lines of code, the overhead is
> 1 spin_try_lock()(fast path) + N __this_cpu_inc()
>
I'm assuming that calculation is no longer valid, considering that
we just discussed how the per-cpu refcnt that you were using is quite
unnecessary and can be removed.
IIUC, the overhead with your code, as per above discussion would be:
1 spin_try_lock() [non-nested] + N read_lock(global_rwlock).
Note that I'm referring to the scenario when hotplug is _not_ happening
(ie., nobody is running writer side code).
> The overhead of your code is
> 2 smp_mb() + N __this_cpu_inc()
>
Right. And as you can see, this is much much better than the overhead
shown above.
> I don't see how much different.
>
>>>> Also, this lg_rwlock implementation uses 3
>>>> different data-structures - a per-cpu spinlock, a global rwlock and
>>>> a per-cpu refcnt, and its not immediately apparent why you need those many
>>>> or even those many varieties.
>>>
>>> data-structures is the same as yours.
>>> fallback_reader_refcnt <--> reader_refcnt
>>
>> This has semantic problems, as noted above.
>>
>>> per-cpu spinlock <--> write_signal
>>
>> Acquire/release of (spin) lock is costlier than inc/dec of a counter, IIUC.
>>
>>> fallback_rwlock <---> global_rwlock
>>>
>>>> Also I see that this doesn't handle the
>>>> case of interrupt-handlers also being readers.
>>>
>>> handled. nested reader will see the ref or take the fallback_rwlock
>>>
>
> Sorry, _reentrance_ read_lock() will see the ref or take the fallback_rwlock
>
>>
>> I'm not referring to simple nested readers here, but interrupt handlers who
>> can act as readers. For starters, the arch_spin_trylock() is not safe when
>> interrupt handlers can also run the same code, right? You'll need to save
>> and restore interrupts at critical points in the code. Also, the __foo()
>> variants used to read/update the counters are not interrupt-safe.
>
> I must missed something.
>
> Could you elaborate more why arch_spin_trylock() is not safe when
> interrupt handlers can also run the same code?
>
> Could you elaborate more why __this_cpu_op variants is not
> interrupt-safe since they are always called paired.
>
Take a look at include/linux/percpu.h. You'll note that __this_cpu_*
operations map to __this_cpu_generic_to_op(), which doesn't disable interrupts
while doing the update. Hence you can get inconsistent results if an interrupt
hits the CPU at that time and the interrupt handler tries to do the same thing.
In contrast, if you use this_cpu_inc() for example, interrupts are explicitly
disabled during the update and hence you won't get inconsistent results.
>
>> And,
>> the unlock() code in the reader path is again going to be confused about
>> what to do when interrupt handlers interrupt regular readers, due to the
>> messed up refcount.
>
> I still can't understand.
>
The primary reason _I_ was using the refcnt vs the reason _you_ were using the
refcnt, appears to be very different. Maybe that's why the above statement
didn't make sense. In your case, IIUC, you can simply get rid of the refcnt
and replace it with the simple check I mentioned above. Whereas, I use
refcnts to keep the reader-side synchronization fast (and for reader-writer
communication).
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [iproute2 ] Fix compilation error of m_ipt.c with -Werror enabled
From: Vijay Subramanian @ 2013-02-26 19:23 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Vijay Subramanian
In-Reply-To: <1361905927-31534-1-git-send-email-subramanian.vijay@gmail.com>
On 26 February 2013 11:12, Vijay Subramanian
<subramanian.vijay@gmail.com> wrote:
> Commit (5a650703d47e10aa386406c855eff5a593b2120b Makefile: make warnings into
> errors ) causes the following build error.
>
A similar error occurs for misc/ss.c
gcc -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes
-Wmissing-declarations -Wold-style-definition -O2
-I../include -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\"
-DCONFDIR=\"/etc/iproute2\" -D_GNU_SOURCE -c -o ss.o ss.c
cc1: warnings being treated as errors
ss.c: In function 'user_ent_hash_build':
ss.c:300: error: ignoring return value of 'fscanf', declared with
attribute warn_unused_result
ss.c: In function 'get_slabstat':
ss.c:382: error: ignoring return value of 'fgets', declared with
attribute warn_unused_result
ss.c: In function 'init_service_resolver':
ss.c:506: error: ignoring return value of 'fgets', declared with
attribute warn_unused_result
---cut---
Not sure what is the best fix for this but can send a patch based on
suggestions.
I am using "Ubuntu 10.04.1 LTS" with ldd (GNU libc) 2.17.
Vijay
^ permalink raw reply
* [iproute2 ] Fix compilation error of m_ipt.c with -Werror enabled
From: Vijay Subramanian @ 2013-02-26 19:12 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, Vijay Subramanian
Commit (5a650703d47e10aa386406c855eff5a593b2120b Makefile: make warnings into
errors ) causes the following build error.
gcc -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes
-Wmissing-declarations -Wold-style-definition -O2 -I../include
-DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\"
-D_GNU_SOURCE -DCONFIG_GACT -DCONFIG_GACT_PROB -DIPT_LIB_DIR=\"/lib/xtables\"
-DYY_NO_INPUT -c -o m_ipt.o m_ipt.c
cc1: warnings being treated as errors
m_ipt.c:72: error: no previous prototype for 'xtables_register_target'
m_ipt.c:361: error: no previous prototype for 'build_st'
make[1]: *** [m_ipt.o] Error 1
This is fixed by adding the prototype in the header include/iptables.h
I am not sure if this is due to something wrong on my build system but I am
using current glibc 2.17.
Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
include/iptables.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/iptables.h b/include/iptables.h
index dd844c1..f1e62e2 100644
--- a/include/iptables.h
+++ b/include/iptables.h
@@ -143,6 +143,8 @@ extern int line;
/* Your shared library should call one of these. */
extern void register_match(struct iptables_match *me);
extern void register_target(struct iptables_target *me);
+extern void xtables_register_target(struct iptables_target *me);
+extern int build_st(struct iptables_target *target, struct ipt_entry_target *t);
extern struct in_addr *dotted_to_addr(const char *dotted);
extern char *addr_to_dotted(const struct in_addr *addrp);
--
1.7.0.4
^ permalink raw reply related
* [PATCH net 3/3] sfc: Detach net device when stopping queues for reconfiguration
From: Ben Hutchings @ 2013-02-26 19:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1361905763.2996.9.camel@bwh-desktop.uk.solarflarecom.com>
We must only ever stop TX queues when they are full or the net device
is not 'ready' so far as the net core, and specifically the watchdog,
is concerned. Otherwise, the watchdog may fire *immediately* if no
packets have been added to the queue in the last 5 seconds.
The device is ready if all the following are true:
(a) It has a qdisc
(b) It is marked present
(c) It is running
(d) The link is reported up
(a) and (c) are normally true, and must not be changed by a driver.
(d) is under our control, but fake link changes may disturb userland.
This leaves (b). We already mark the device absent during reset
and self-test, but we need to do the same during MTU changes and ring
reallocation. We don't need to do this when the device is brought
down because then (c) is already false.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index bf57b3c..0bc0099 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -779,6 +779,7 @@ efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
tx_queue->txd.entries);
}
+ efx_device_detach_sync(efx);
efx_stop_all(efx);
efx_stop_interrupts(efx, true);
@@ -832,6 +833,7 @@ out:
efx_start_interrupts(efx, true);
efx_start_all(efx);
+ netif_device_attach(efx->net_dev);
return rc;
rollback:
@@ -1641,8 +1643,12 @@ static void efx_stop_all(struct efx_nic *efx)
/* Flush efx_mac_work(), refill_workqueue, monitor_work */
efx_flush_all(efx);
- /* Stop the kernel transmit interface late, so the watchdog
- * timer isn't ticking over the flush */
+ /* Stop the kernel transmit interface. This is only valid if
+ * the device is stopped or detached; otherwise the watchdog
+ * may fire immediately.
+ */
+ WARN_ON(netif_running(efx->net_dev) &&
+ netif_device_present(efx->net_dev));
netif_tx_disable(efx->net_dev);
efx_stop_datapath(efx);
@@ -1963,16 +1969,18 @@ static int efx_change_mtu(struct net_device *net_dev, int new_mtu)
if (new_mtu > EFX_MAX_MTU)
return -EINVAL;
- efx_stop_all(efx);
-
netif_dbg(efx, drv, efx->net_dev, "changing MTU to %d\n", new_mtu);
+ efx_device_detach_sync(efx);
+ efx_stop_all(efx);
+
mutex_lock(&efx->mac_lock);
net_dev->mtu = new_mtu;
efx->type->reconfigure_mac(efx);
mutex_unlock(&efx->mac_lock);
efx_start_all(efx);
+ netif_device_attach(efx->net_dev);
return 0;
}
--
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 related
* [PATCH net 2/3] sfc: Fix efx_rx_buf_offset() in the presence of swiotlb
From: Ben Hutchings @ 2013-02-26 19:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1361905763.2996.9.camel@bwh-desktop.uk.solarflarecom.com>
We assume that the mapping between DMA and virtual addresses is done
on whole pages, so we can find the page offset of an RX buffer using
the lower bits of the DMA address. However, swiotlb maps in units of
2K, breaking this assumption.
Add an explicit page_offset field to struct efx_rx_buffer.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/net_driver.h | 4 +++-
drivers/net/ethernet/sfc/rx.c | 10 +++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 2d756c1..0a90abd 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -210,6 +210,7 @@ struct efx_tx_queue {
* Will be %NULL if the buffer slot is currently free.
* @page: The associated page buffer. Valif iff @flags & %EFX_RX_BUF_PAGE.
* Will be %NULL if the buffer slot is currently free.
+ * @page_offset: Offset within page. Valid iff @flags & %EFX_RX_BUF_PAGE.
* @len: Buffer length, in bytes.
* @flags: Flags for buffer and packet state.
*/
@@ -219,7 +220,8 @@ struct efx_rx_buffer {
struct sk_buff *skb;
struct page *page;
} u;
- unsigned int len;
+ u16 page_offset;
+ u16 len;
u16 flags;
};
#define EFX_RX_BUF_PAGE 0x0001
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index a575491..879ff58 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -90,11 +90,7 @@ static unsigned int rx_refill_threshold;
static inline unsigned int efx_rx_buf_offset(struct efx_nic *efx,
struct efx_rx_buffer *buf)
{
- /* Offset is always within one page, so we don't need to consider
- * the page order.
- */
- return ((unsigned int) buf->dma_addr & (PAGE_SIZE - 1)) +
- efx->type->rx_buffer_hash_size;
+ return buf->page_offset + efx->type->rx_buffer_hash_size;
}
static inline unsigned int efx_rx_buf_size(struct efx_nic *efx)
{
@@ -187,6 +183,7 @@ static int efx_init_rx_buffers_page(struct efx_rx_queue *rx_queue)
struct efx_nic *efx = rx_queue->efx;
struct efx_rx_buffer *rx_buf;
struct page *page;
+ unsigned int page_offset;
struct efx_rx_page_state *state;
dma_addr_t dma_addr;
unsigned index, count;
@@ -211,12 +208,14 @@ static int efx_init_rx_buffers_page(struct efx_rx_queue *rx_queue)
state->dma_addr = dma_addr;
dma_addr += sizeof(struct efx_rx_page_state);
+ page_offset = sizeof(struct efx_rx_page_state);
split:
index = rx_queue->added_count & rx_queue->ptr_mask;
rx_buf = efx_rx_buffer(rx_queue, index);
rx_buf->dma_addr = dma_addr + EFX_PAGE_IP_ALIGN;
rx_buf->u.page = page;
+ rx_buf->page_offset = page_offset;
rx_buf->len = efx->rx_buffer_len - EFX_PAGE_IP_ALIGN;
rx_buf->flags = EFX_RX_BUF_PAGE;
++rx_queue->added_count;
@@ -227,6 +226,7 @@ static int efx_init_rx_buffers_page(struct efx_rx_queue *rx_queue)
/* Use the second half of the page */
get_page(page);
dma_addr += (PAGE_SIZE >> 1);
+ page_offset += (PAGE_SIZE >> 1);
++count;
goto split;
}
--
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 related
* [PATCH net 1/3] sfc: Properly sync RX DMA buffer when it is not the last in the page
From: Ben Hutchings @ 2013-02-26 19:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
In-Reply-To: <1361905763.2996.9.camel@bwh-desktop.uk.solarflarecom.com>
We may currently allocate two RX DMA buffers to a page, and only unmap
the page when the second is completed. We do not sync the first RX
buffer to be completed; this can result in packet loss or corruption
if the last RX buffer completed in a NAPI poll is the first in a page
and is not DMA-coherent. (In the middle of a NAPI poll, we will
handle the following RX completion and unmap the page *before* looking
at the content of the first buffer.)
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/rx.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index d780a0d..a575491 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -236,7 +236,8 @@ static int efx_init_rx_buffers_page(struct efx_rx_queue *rx_queue)
}
static void efx_unmap_rx_buffer(struct efx_nic *efx,
- struct efx_rx_buffer *rx_buf)
+ struct efx_rx_buffer *rx_buf,
+ unsigned int used_len)
{
if ((rx_buf->flags & EFX_RX_BUF_PAGE) && rx_buf->u.page) {
struct efx_rx_page_state *state;
@@ -247,6 +248,10 @@ static void efx_unmap_rx_buffer(struct efx_nic *efx,
state->dma_addr,
efx_rx_buf_size(efx),
DMA_FROM_DEVICE);
+ } else if (used_len) {
+ dma_sync_single_for_cpu(&efx->pci_dev->dev,
+ rx_buf->dma_addr, used_len,
+ DMA_FROM_DEVICE);
}
} else if (!(rx_buf->flags & EFX_RX_BUF_PAGE) && rx_buf->u.skb) {
dma_unmap_single(&efx->pci_dev->dev, rx_buf->dma_addr,
@@ -269,7 +274,7 @@ static void efx_free_rx_buffer(struct efx_nic *efx,
static void efx_fini_rx_buffer(struct efx_rx_queue *rx_queue,
struct efx_rx_buffer *rx_buf)
{
- efx_unmap_rx_buffer(rx_queue->efx, rx_buf);
+ efx_unmap_rx_buffer(rx_queue->efx, rx_buf, 0);
efx_free_rx_buffer(rx_queue->efx, rx_buf);
}
@@ -535,10 +540,10 @@ void efx_rx_packet(struct efx_rx_queue *rx_queue, unsigned int index,
goto out;
}
- /* Release card resources - assumes all RX buffers consumed in-order
- * per RX queue
+ /* Release and/or sync DMA mapping - assumes all RX buffers
+ * consumed in-order per RX queue
*/
- efx_unmap_rx_buffer(efx, rx_buf);
+ efx_unmap_rx_buffer(efx, rx_buf, len);
/* Prefetch nice and early so data will (hopefully) be in cache by
* the time we look at it.
--
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 related
* Pull request: sfc 2013-02-26
From: Ben Hutchings @ 2013-02-26 19:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers, scrum-linux
The following changes since commit eb970ff07c15f13eb474f643fd165ebe3e4e24b2:
usbnet: smsc95xx: rename FEATURE_AUTOSUSPEND (2013-02-25 15:49:52 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc.git sfc-3.9
Some fixes that should go into 3.9:
1. Fix packet corruption when using non-coherent RX DMA buffers.
2. Fix occasional watchdog misfiring when changing MTU or ring size.
These are longstanding bugs and should be fixed in stable as well, but
I'd like to review other recent fixes first and send a separate request
for stable inclusion.
Ben.
Ben Hutchings (3):
sfc: Properly sync RX DMA buffer when it is not the last in the page
sfc: Fix efx_rx_buf_offset() in the presence of swiotlb
sfc: Detach net device when stopping queues for reconfiguration
drivers/net/ethernet/sfc/efx.c | 16 ++++++++++++----
drivers/net/ethernet/sfc/net_driver.h | 4 +++-
drivers/net/ethernet/sfc/rx.c | 25 +++++++++++++++----------
3 files changed, 30 insertions(+), 15 deletions(-)
--
Ben Hutchings, Staff Engineer, Solarflare
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: Fw: [Bug 54231] r8169 driver regression caused by the commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd
From: Tomi Orava @ 2013-02-26 18:41 UTC (permalink / raw)
To: Francois Romieu; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20130224220413.GA22870@electric-eye.fr.zoreil.com>
On 02/25/2013 12:04 AM, Francois Romieu wrote:
> Tomi Orava <tomimo@ncircle.nullnet.fi> :
>> On 02/23/2013 01:09 AM, Francois Romieu wrote:
> [...]
>> I started re-testing after your comment and figured out that the
>> real problem seems to lie somehow with jumbo frames. Ie. the DMA burst
>> changes do not actually prevent the hangs at all in my case. The
>> catch here, that I missed previously, was that for some interesting
>> reason the NIC will fail in a couple of minutes with a suitable traffic
>> if the jumbo frames (mtu 4000) have been enabled from the start.
>> However, if I enable the jumbo frames manually after the system
>> has already started up, there are no stability issues related to network.
>
> If you mean that 'ip link set dev ... mtu 4000; ip link set dev ... up'
> fails whereas 'ip link set dev ... up; ip link set dev ... mtu 4000'
> works, the patch below is a candidate:
Yes, this fits exactly my problem. I tested this change on 3.7.9 and
on 3.4.33 and I had no network hangs anymore.
Thanks!
Tomi
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 8900398..af99498 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -4766,7 +4766,7 @@ static void rtl_hw_start_8168bb(struct rtl8169_private *tp)
> RTL_W16(CPlusCmd, RTL_R16(CPlusCmd) & ~R8168_CPCMD_QUIRK_MASK);
>
> rtl_tx_performance_tweak(pdev,
> - (0x5 << MAX_READ_REQUEST_SHIFT) | PCI_EXP_DEVCTL_NOSNOOP_EN);
> + (0x2 << MAX_READ_REQUEST_SHIFT) | PCI_EXP_DEVCTL_NOSNOOP_EN);
> }
>
> static void rtl_hw_start_8168bef(struct rtl8169_private *tp)
> ---
>
> [...]
>>> Tomi, what does lspci say about your 8168b device ?
>>
>> The NIC information is:
>>
>> 03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI
>> Express Gigabit Ethernet controller (rev 01)
>>
>> 03:00.0 0200: 10ec:8168 (rev 01)
>
> Ok, so the first hunk of the patch that Stephen forwarded would not make
> any difference for your chipset.
>
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: David Miller @ 2013-02-26 17:43 UTC (permalink / raw)
To: vyasevic; +Cc: netdev
In-Reply-To: <1361897970-1728-1-git-send-email-vyasevic@redhat.com>
No new ioctls please, extend the netlink interfaces as necessary
instead.
Thanks.
^ 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