* Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Daniel Borkmann @ 2014-09-18 14:50 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuyZCHh7SGeLF+g0ye3OrEFWnTZCn3SBeK0-8qz-y8U30w@mail.gmail.com>
On 09/18/2014 04:34 PM, Alexei Starovoitov wrote:
> On Wed, Sep 17, 2014 at 11:44 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
...
>> Sure, you will never get a full compatibility on that regard
>> while backwards compatibility needs to be guaranteed on the
>> other hand. I looked at perf_copy_attr() implementation and I
>> think that we should mimic it in a very similar way as it
>> exactly solves what we need.
>>
>> For example, it will return with -EINVAL for (size > PAGE_SIZE)
>> and (size < PERF_ATTR_SIZE_VER0) where PAGE_SIZE has been chosen
>> as an arbitrary hard upper limit where it is believed that it will
>> never grow beyond that large limit in future.
>>
>> So this is a more loose constraint than what we currently do,
>> that is, -EINVAL on (size > sizeof(attr)) where attr is the
>> currently known size of a specific kernel. That would at least
>> be a start, you won't be able to cover everything though, but
>> it would allow to address the issue raised when running with
>> a basic feature set.
>
> you missed my point. We should not 'do a start', since it
> doesn't help user space in the long run and only makes
> kernel more complex.
Sorry, I don't think I missed your point. But if you see things
differently, fair enough, it was just a suggestion.
^ permalink raw reply
* Re: [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected
From: Giuseppe CAVALLARO @ 2014-09-18 14:49 UTC (permalink / raw)
To: Kweh Hock Leong, David S. Miller, rayagond
Cc: Vince Bridgers, Chen-Yu Tsai, netdev, LKML, Ong Boon Leong,
Tobias Klausmann
In-Reply-To: <1411043650-31712-1-git-send-email-hock.leong.kweh@intel.com>
On 9/18/2014 2:34 PM, Kweh Hock Leong wrote:
> From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
>
> When the CONFIG_HAVE_CLK is selected for the system, the stmmac_pci_probe
> will fail with dmesg:
> [ 2.167225] stmmaceth 0000:00:14.6: enabling device (0000 -> 0002)
> [ 2.178267] stmmaceth 0000:00:14.6: enabling bus mastering
> [ 2.178436] stmmaceth 0000:00:14.6: irq 24 for MSI/MSI-X
> [ 2.178703] stmmaceth 0000:00:14.6: stmmac_dvr_probe: warning: cannot
> get CSR clock
> [ 2.186503] stmmac_pci_probe: main driver probe failed
> [ 2.194003] stmmaceth 0000:00:14.6: disabling bus mastering
> [ 2.196473] stmmaceth: probe of 0000:00:14.6 failed with error -2
>
> This patch fix the issue by breaking the dependency to devm_clk_get()
> as the CSR clock can be obtained at priv->plat->clk_csr from pci driver.
>
> Reported-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
> Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 08addd6..ea3859a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2714,10 +2714,15 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
>
> priv->stmmac_clk = devm_clk_get(priv->device, STMMAC_RESOURCE_NAME);
> if (IS_ERR(priv->stmmac_clk)) {
> - dev_warn(priv->device, "%s: warning: cannot get CSR clock\n",
Hmm I am not sure this is the right fix. The driver has to fail if the
main clock is not found. Indeed dev_warn has to be changed in dev_err.
Take a look at Documentation/networking/stmmac.txt but I will post some
patch to improve the documentation adding further detail for clocks too.
The the logic behind the code is that the CSR clock will be set at
runtime if in case of priv->plat->clk_csr ==0 or it will be forced to
a fixed value if passed from the platform instead of.
IIRC This was required on some platforms time ago.
For sure the driver is designed to fail in case of no main clock is
found.
Peppe
> - __func__);
> - ret = PTR_ERR(priv->stmmac_clk);
> - goto error_clk_get;
> + if (!priv->plat->clk_csr) {
> + dev_warn(priv->device,
> + "%s: warning: cannot get CSR clock\n",
> + __func__);
> + ret = PTR_ERR(priv->stmmac_clk);
> + goto error_clk_get;
> + } else {
> + priv->stmmac_clk = NULL;
> + }
> }
> clk_prepare_enable(priv->stmmac_clk);
>
>
^ permalink raw reply
* Re: [PATCH net-next 1/8] flow_keys: Record IP layer protocol in skb_flow_dissect()
From: Eric Dumazet @ 2014-09-18 14:38 UTC (permalink / raw)
To: Or Gerlitz
Cc: Govindarajulu Varadarajan, Yinghai Lu, David Miller, NetDev,
ssujith, gvaradar, Christian Benvenuti (benve)
In-Reply-To: <CAJZOPZJ7+n0t1LCqaa3JCBByRw=f_Tp6rk8vm0ZnCzmtKFKX_g@mail.gmail.com>
On Thu, 2014-09-18 at 17:18 +0300, Or Gerlitz wrote:
> The crash happens 100% on IPoIB (IP-over-Infiniband) [1] interfaces
> b/c your upstream commit e0f31d8 "flow_keys: Record IP layer protocol
> in skb_flow_dissect()" causes the IPoIB data stashed on skb->cb [2] to
> smash other skb fields.
>
> So your 3.17-rc1 commit introduced a regression to how things work
> since kernel 3.2
>
> Can please see how to revert this hunk
>
> -- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -231,7 +231,7 @@ struct qdisc_skb_cb {
> unsigned int pkt_len;
> u16 slave_dev_queue_mapping;
> u16 _pad;
> - unsigned char data[20];
> + unsigned char data[24];
> };
>
> thanks,
>
> Or.
>
> [1] http://marc.info/?l=linux-rdma&m=141029109017035&w=2
> [2] see these commits
>
> 936d7de3 IPoIB: Stop lying about hard_header_len and use skb->cb to
> stash LL addresses
> a0417fa3 net: Make qdisc_skb_cb upper size bound explicit
I am taking care of this right now guys.
^ permalink raw reply
* Re: [PATCH v13 net-next 07/11] bpf: verifier (add ability to receive verification log)
From: Alexei Starovoitov @ 2014-09-18 14:34 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <541A7F35.2010804@redhat.com>
On Wed, Sep 17, 2014 at 11:44 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/18/2014 01:45 AM, Alexei Starovoitov wrote:
>>
>> On Wed, Sep 17, 2014 at 12:37 PM, Alexei Starovoitov <ast@plumgrid.com>
>> wrote:
>>>
>>>
>>>> Hm, thinking out loudly ... perhaps this could be made a library
>>>> problem.
>>>> Such that the library which wraps the syscall needs to be aware of a
>>>> marker where the initial version ends, and if the application doesn't
>>>> make use of any of the new features, it would just pass in the length up
>>>> to the marker as size attribute into the syscall. Similarly, if new
>>>> features are always added to the end of a structure and the library
>>>> truncates the overall-length after the last used member we might have
>>>> a chance to load something on older kernels, haven't tried that though.
>>>
>>>
>>> that's a 3rd option. I think it's cleaner than 2nd, since it solves it
>>> completely from user space.
>>> It can even be smarter than that. If this syscall wrapper library
>>> sees that newer features are used and it can workaround them:
>>> it can chop size and pass older fields into the older kernel
>>> and when it returns, do a workaround based on newer fields.
>>
>>
>> the more I think about 'new user space + old kernel' problem,
>> the more certain I am that kernel should not try to help
>> user space, since most of the time it's not going to be enough,
>> but additional code in kernel would need to be maintained.
>>
>> syscall commands and size of bpf_attr is the least of problems.
>> New map_type and prog_type will be added, new helper
>> functions will be available to programs.
>> One would think that md5 of uapi/linux/bpf.h would be
>> enough to say that user app is compatible... In reality,
>> it's not. The 'state pruning' verifier optimization I've talked
>> about will not change a single bit in bpf.h, but it will be
>> able to recognize more programs as safe.
>> A program developed on a new kernel with more
>> advanced verifier will load just fine on new kernel, but
>> this valid program will not load on old kernel, only because
>> verifier is not smart enough. Now we would need a version
>> of verifier exposed all the way to user space?!
>> imo that's too much. I think for eBPF infra kernel
>> should only guarantee backwards compatibility
>> (old user space must work with new kernel) and that's it.
>> That's what this patch is trying to do.
>> Thoughts?
>
>
> Sure, you will never get a full compatibility on that regard
> while backwards compatibility needs to be guaranteed on the
> other hand. I looked at perf_copy_attr() implementation and I
> think that we should mimic it in a very similar way as it
> exactly solves what we need.
>
> For example, it will return with -EINVAL for (size > PAGE_SIZE)
> and (size < PERF_ATTR_SIZE_VER0) where PAGE_SIZE has been chosen
> as an arbitrary hard upper limit where it is believed that it will
> never grow beyond that large limit in future.
>
> So this is a more loose constraint than what we currently do,
> that is, -EINVAL on (size > sizeof(attr)) where attr is the
> currently known size of a specific kernel. That would at least
> be a start, you won't be able to cover everything though, but
> it would allow to address the issue raised when running with
> a basic feature set.
you missed my point. We should not 'do a start', since it
doesn't help user space in the long run and only makes
kernel more complex.
^ permalink raw reply
* [PATCH] tg3: Work around HW/FW limitations with vlan encapsulated frames
From: Vladislav Yasevich @ 2014-09-18 14:31 UTC (permalink / raw)
To: netdev; +Cc: Vladislav Yasevich, Prashant Sreedharan, Michael Chan
TG3 appears to have an issue performing TSO and checksum offloading
correclty when the frame has been vlan encapsulated (non-accelrated).
In these cases, tcp checksum is not correctly updated.
This patch attempts to work around this issue. After the patch,
802.1ad vlans start working correctly over tg3 devices.
CC: Prashant Sreedharan <prashant@broadcom.com>
CC: Michael Chan <mchan@broadcom.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
drivers/net/ethernet/broadcom/tg3.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index cb77ae9..e7d3a62 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -7914,8 +7914,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
entry = tnapi->tx_prod;
base_flags = 0;
- if (skb->ip_summed == CHECKSUM_PARTIAL)
- base_flags |= TXD_FLAG_TCPUDP_CSUM;
mss = skb_shinfo(skb)->gso_size;
if (mss) {
@@ -7929,6 +7927,13 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb) - ETH_HLEN;
+ /* HW/FW can not correctly segment packets that have been
+ * vlan encapsulated.
+ */
+ if (skb->protocol == htons(ETH_P_8021Q) ||
+ skb->protocol == htons(ETH_P_8021AD))
+ return tg3_tso_bug(tp, tnapi, txq, skb);
+
if (!skb_is_gso_v6(skb)) {
if (unlikely((ETH_HLEN + hdr_len) > 80) &&
tg3_flag(tp, TSO_BUG))
@@ -7979,6 +7984,17 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
base_flags |= tsflags << 12;
}
}
+ } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ /* HW/FW can not correctly checksum packets that have been
+ * vlan encapsulated.
+ */
+ if (skb->protocol == htons(ETH_P_8021Q) ||
+ skb->protocol == htons(ETH_P_8021AD)) {
+ if (skb_checksum_help(skb))
+ goto drop;
+ } else {
+ base_flags |= TXD_FLAG_TCPUDP_CSUM;
+ }
}
if (tg3_flag(tp, USE_JUMBO_BDFLAG) &&
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 1/2 v2 nf-next] net: bridge: don't register netfilter call_iptables hooks by default
From: Florian Westphal @ 2014-09-18 14:31 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel, netdev
In-Reply-To: <20140918133511.GA8740@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Sep 17, 2014 at 05:52:16PM +0200, Florian Westphal wrote:
> > Jesper reports that kernels with CONFIG_BRIDGE_NETFILTER=n show significantly
> > better performance vs. CONFIG_BRIDGE_NETFILTER=y, even with
> > bridge-nf-call-iptables=0.
> >
> > This is because bridge registers some bridge netfilter hooks at
> > module load time, so the static key to bypass nf rule evaluation
> > via NF_HOOK() is false.
>
> I'm attaching a patch to decouple bridge netfilter from the bridge
> core. The idea is to encapsulate the hook registration and most of
> its code in a separated module: br_netfilter.
Right, thats better.
I did not do this since it means modprobe bridge.ko will no longer
register the needed sysctls, and I deemed autoloading pointless
since that means the hooks are registered on bridge.ko load.
Also, it seems to be that we will have to wait forever to eventually
get rid of the autoload...
> I also guess most distributors will use CONFIG_BRIDGE_NETFILTER=m.
> Then, users will get a warning message to let them know that they will
> have to modprobe br_netfilter in the future if they need it, so we can
> remove the deferred request_module from the br init path.
Hmm, not sure if its safe to do this, e.g. with bridge=y,
br_netfilter=m, module might not yet be present in such cases?
Also, what about this:
iptables-restore < rules.txt
modprobe bridge
brctl addbr ...
brctl addif ...
at this point, any packet forwarded by bridge is filtered
via iptables.
After your patch, this might no longer be the case, if the modprobe
call is delayed (maybe this is far-fetched and not an issue in practice)?
2nd hypothetical issue:
modprobe bridge
sysctl net.bridge.bridge-nf-call-iptables=0
The sysctl could fail when br_nf_core is not yet present.
> Unless I'm missing anything, I think br_netfilter should have been a
> separated module since the beginning. The hook registration in other
> netfilter modules is also ruled by rmmod/modprobe, so better if we
> recover that path in this code.
Agreed.
> Kconfig (bridge=y, br_netfilter=m) and so on, btw. I also may include
> part of your original patch description if you're OK with it.
Sure, go ahead.
> Please, let me know if I overlook anything. Thanks.
> This patch modularizes br_netfilter so it can be
> rmmod'ed, thus, the hooks can be unregistered.
I see. Yes, that makes sense.
Another alternative would be to merge both approaches.
I.e, use my patch, but move all the hook registration to
your proposed br_nf_core module.
The sysctls would still be registered from bridge.ko.
Then, if call_iptables is set and iptables rules are active,
do the module autoload and print the warning from your patch, telling
people to modprobe br_nf_core if they want the filtering.
Unfortunately, we'd have to export (from bridge.ko) some hook
to allow br_nf_core to signal that the hooks have been registered.
Then, in two years or so, we would remove the autoload hook
in the packet procesing path.
The only other disadvantage that I see is that we'd still have
the bridge-nf-* sysctls in bridge.ko rather than the new glue module.
^ permalink raw reply
* Re: [PATCH net-next 1/8] flow_keys: Record IP layer protocol in skb_flow_dissect()
From: Or Gerlitz @ 2014-09-18 14:18 UTC (permalink / raw)
To: Govindarajulu Varadarajan
Cc: Yinghai Lu, David Miller, NetDev, ssujith, gvaradar,
Christian Benvenuti (benve)
In-Reply-To: <alpine.LNX.2.03.1406261159071.1438@ws.cisco>
On Thu, Jun 26, 2014 at 9:34 AM, Govindarajulu Varadarajan
<_govind@gmx.com> wrote:
>
>
> On Mon, 23 Jun 2014, Yinghai Lu wrote:
>>
>> this patch in net-next cause kernel crash.
>>
>> [ 148.466045] qlge 0000:4a:00.1 eth27: Passed Get Port Configuration.
>> [ 162.385445] BUG: unable to handle kernel paging request at 000000010000007e
>> [ 162.385839] IP: [<ffffffff81f18899>] __dev_queue_xmit+0x399/0x630
>> [ 162.398541] PGD 0
>> [ 162.398659] Oops: 0002 [#1] SMP
>> [ 162.398845] Modules linked in:
>> [ 162.399022] CPU: 5 PID: 1 Comm: swapper/0 Tainted: G W
>> 3.16.0-rc2-yh-00302-g3d5dc41-dirty #22
>> [ 162.418490] Hardware name: Oracle Corporation unknown /
>> , BIOS 11016600 05/17/2011
>> [ 162.438851] task: ffff884027a80000 ti: ffff881027d20000 task.ti:
>> ffff881027d20000
>> [ 162.468329] RIP: 0010:[<ffffffff81f18899>] [<ffffffff81f18899>]
>> __dev_queue_xmit+0x399/0x630
>> [ 162.488085] RSP: 0000:ffff881027d23d28 EFLAGS: 00010202
>> [ 162.488345] RAX: 00000000fffffffe RBX: ffff887026041000 RCX: 0000000000000001
>> [ 162.508245] RDX: 0000000000000000 RSI: ffffffff82dfee78 RDI: ffff884027a80000
>> [ 162.508590] RBP: ffff881027d23d70 R08: 0000000000000001 R09: 0000000000000000
>> [ 162.528255] R10: 0000000000000000 R11: ffff885026020800 R12: ffffffff82dfedc0
>> [ 162.547963] R13: ffff881022b7c000 R14: 0000000000000dac R15: ffff88702434c400
>> [ 162.548310] FS: 0000000000000000(0000) GS:ffff88103f000000(0000)
>> knlGS:0000000000000000
>> [ 162.568186] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> [ 162.568489] CR2: 000000010000007e CR3: 0000000002c19000 CR4: 00000000000007e0
>> [ 162.588263] Stack:
>> [ 162.588354] ffffffff81f18505 0000000000000000 00ff881027d23d80
>> ffffffff82dfee60
>> [ 162.608269] ffff885023dd1e40 ffff881022b7c000 ffff885026020929
>> 0000000000000dac
>> [ 162.628043] ffff887026041000 ffff881027d23d80 ffffffff81f18b40
>> ffff881027d23e18
>> [ 162.628422] Call Trace:
>> [ 162.647963] [<ffffffff81f18505>] ? __dev_queue_xmit+0x5/0x630
>> [ 162.648284] [<ffffffff81f18b40>] dev_queue_xmit+0x10/0x20
>> [ 162.667987] [<ffffffff83087b64>] ip_auto_config+0x8e6/0xf13
>> [ 162.668282] [<ffffffff8100031d>] ? do_one_initcall+0xdd/0x1e0
>> [ 162.688018] [<ffffffff810e36ad>] ? trace_hardirqs_on+0xd/0x10
>> [ 162.688298] [<ffffffff8110c40f>] ? ktime_get+0xbf/0x140
>> [ 162.708029] [<ffffffff8308727e>] ? root_nfs_parse_addr+0xbd/0xbd
>> [ 162.708292] [<ffffffff81000323>] do_one_initcall+0xe3/0x1e0
>> [ 162.728075] [<ffffffff810ba8bd>] ? parse_args+0x1ed/0x330
>> [ 162.728340] [<ffffffff8203501a>] ? printk+0x54/0x56
>> [ 162.748027] [<ffffffff8301f4c5>] kernel_init_freeable+0x237/0x2ce
>> [ 162.748344] [<ffffffff8301eaf7>] ? do_early_param+0x8a/0x8a
>> [ 162.768070] [<ffffffff8202abb0>] ? rest_init+0xc0/0xc0
>> [ 162.768318] [<ffffffff8202abbe>] kernel_init+0xe/0x100
>> [ 162.788057] [<ffffffff8204c4ac>] ret_from_fork+0x7c/0xb0
>> [ 162.788314] [<ffffffff8202abb0>] ? rest_init+0xc0/0xc0
>> [ 162.808008] Code: e8 5d 48 18 ff eb 13 48 c7 c7 60 d9 c5 82 e8 cf
>> bc 1c ff 85 c0 74 dd 0f 1f 00 48 8b 43 58 48 83 e0 fe 48 85 c0 48 89
>> 43 58 74 07 <f0> ff 80 80 00 00 00 4c 89 e6 48 89 df 41 ff 14 24 41 89
>> c6 41
>> [ 162.828797] RIP [<ffffffff81f18899>] __dev_queue_xmit+0x399/0x630
>> [ 162.848186] RSP <ffff881027d23d28>
>> [ 162.848341] CR2: 000000010000007e
>> [ 162.848490] ---[ end trace 26b7736a09036e46 ]---
>> [ 162.868194] Kernel panic - not syncing: Fatal exception in interrupt
>> [ 162.872673] Kernel Offset: 0x0 from 0xffffffff81000000 (relocation
>> range: 0xffffffff80000000-0xffffffff9fffffff)
>> [ 162.888531] ---[ end Kernel panic - not syncing: Fatal exception in interrupt
>> [ 162.908306] ------------[ cut here ]------------
>>
>> After the commit is reverted, the system work again.
>
>
> I do not see any problem in my system.
>
> Did you try disecting what "__dev_queue_xmit+0x399/0x630" is?
>
> On what interface did the crash occur on? is it bond interface?
>
The crash happens 100% on IPoIB (IP-over-Infiniband) [1] interfaces
b/c your upstream commit e0f31d8 "flow_keys: Record IP layer protocol
in skb_flow_dissect()" causes the IPoIB data stashed on skb->cb [2] to
smash other skb fields.
So your 3.17-rc1 commit introduced a regression to how things work
since kernel 3.2
Can please see how to revert this hunk
-- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -231,7 +231,7 @@ struct qdisc_skb_cb {
unsigned int pkt_len;
u16 slave_dev_queue_mapping;
u16 _pad;
- unsigned char data[20];
+ unsigned char data[24];
};
thanks,
Or.
[1] http://marc.info/?l=linux-rdma&m=141029109017035&w=2
[2] see these commits
936d7de3 IPoIB: Stop lying about hard_header_len and use skb->cb to
stash LL addresses
a0417fa3 net: Make qdisc_skb_cb upper size bound explicit
^ permalink raw reply
* Re: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Eric Dumazet @ 2014-09-18 13:41 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexander Duyck, netdev@vger.kernel.org, Tom Herbert
In-Reply-To: <1411047209.7106.255.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, 2014-09-18 at 06:33 -0700, Eric Dumazet wrote:
> Note that for TCP, there are packets sent by the process doing the
> send(), or packets sent by cpu doing TX completion (because of TSQ),
> but also packets sent by ACK processing done in the reverse way.
>
> As Alexander explained, if the ACK packets are delivered into another
> CPU, then you might select another TX queue.
>
> This is mostly prevented because of ooo_okay logic, meaning that a busy
> bulk flow should stick into a single TX queue, no matter of XPS says.
>
> A TCP_RR workload is free to chose whatever queue, because every packet
> starting a RR block has the ooo_okay set (As prior data was delivered
> and acknowledged by the opposite peer)
>
Last but not least, there is the fact that networking stacks use
mod_timer() to arm timers, and that by default, timer migration is on
( cf /proc/sys/kernel/timer_migration )
We probably should use mod_timer_pinned(), but I could not really see
any difference.
^ permalink raw reply
* Re: [PATCH 1/2 v2 nf-next] net: bridge: don't register netfilter call_iptables hooks by default
From: Pablo Neira Ayuso @ 2014-09-18 13:35 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, netdev
In-Reply-To: <1410969137-11885-1-git-send-email-fw@strlen.de>
[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]
Hi Florian,
On Wed, Sep 17, 2014 at 05:52:16PM +0200, Florian Westphal wrote:
> Jesper reports that kernels with CONFIG_BRIDGE_NETFILTER=n show significantly
> better performance vs. CONFIG_BRIDGE_NETFILTER=y, even with
> bridge-nf-call-iptables=0.
>
> This is because bridge registers some bridge netfilter hooks at
> module load time, so the static key to bypass nf rule evaluation
> via NF_HOOK() is false.
I'm attaching a patch to decouple bridge netfilter from the bridge
core. The idea is to encapsulate the hook registration and most of
its code in a separated module: br_netfilter. The bridge core will
still request this module to be loaded from the initialization path,
I think this will retain backward compatibility for users.
I also guess most distributors will use CONFIG_BRIDGE_NETFILTER=m.
Then, users will get a warning message to let them know that they will
have to modprobe br_netfilter in the future if they need it, so we can
remove the deferred request_module from the br init path.
The patch is slightly larger than yours, and I can probably split it
in two patches (one to move several skb-nf-bridge related functions as
static inline to prepare the one that decouples br_netfilter from
bridge).
Unless I'm missing anything, I think br_netfilter should have been a
separated module since the beginning. The hook registration in other
netfilter modules is also ruled by rmmod/modprobe, so better if we
recover that path in this code.
I still have to check later again for typical kbuild robot reports on
Kconfig (bridge=y, br_netfilter=m) and so on, btw. I also may include
part of your original patch description if you're OK with it.
Please, let me know if I overlook anything. Thanks.
[-- Attachment #2: 0001-netfilter-move-br_netfilter-out-of-the-bridge-core.patch --]
[-- Type: text/x-diff, Size: 21736 bytes --]
>From b4b2d36f8b4b1ab1737efc534c910f6211b4571a Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 18 Sep 2014 11:29:03 +0200
Subject: [PATCH] netfilter: move br_netfilter out of the bridge core
Jesper reported that br_netfilter always registers the hooks
since it is part of the bridge core. This harms performance
for people that don't need this.
This patch modularizes br_netfilter so it can be rmmod'ed, thus,
the hooks can be unregistered.
To retain backward compatibility, the bridge core will request
br_netfilter from the initialization path. I guess most
distributors will compile br_netfilter as a module. We also warn
that this automatic br_netfilter module load will be disabled
soon, so users have to update their scripts to modprobe it.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/linux/netfilter_bridge.h | 50 +++++++++++---
include/linux/skbuff.h | 12 ++--
include/net/neighbour.h | 2 +-
include/net/netfilter/ipv4/nf_reject.h | 2 +-
include/net/netfilter/ipv6/nf_reject.h | 2 +-
net/Kconfig | 2 +-
net/bridge/Makefile | 5 +-
net/bridge/br.c | 44 ++++++++++--
net/bridge/br_device.c | 4 +-
net/bridge/br_forward.c | 2 +
net/bridge/br_input.c | 1 +
net/bridge/br_netfilter.c | 116 +++++---------------------------
net/bridge/br_netlink.c | 2 +-
net/bridge/br_nf_core.c | 96 ++++++++++++++++++++++++++
net/bridge/br_private.h | 12 ++--
net/bridge/br_sysfs_br.c | 4 +-
16 files changed, 216 insertions(+), 140 deletions(-)
create mode 100644 net/bridge/br_nf_core.c
diff --git a/include/linux/netfilter_bridge.h b/include/linux/netfilter_bridge.h
index 8ab1c27..c755e49 100644
--- a/include/linux/netfilter_bridge.h
+++ b/include/linux/netfilter_bridge.h
@@ -15,7 +15,7 @@ enum nf_br_hook_priorities {
NF_BR_PRI_LAST = INT_MAX,
};
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
#define BRNF_PKT_TYPE 0x01
#define BRNF_BRIDGED_DNAT 0x02
@@ -24,16 +24,6 @@ enum nf_br_hook_priorities {
#define BRNF_8021Q 0x10
#define BRNF_PPPoE 0x20
-/* Only used in br_forward.c */
-int nf_bridge_copy_header(struct sk_buff *skb);
-static inline int nf_bridge_maybe_copy_header(struct sk_buff *skb)
-{
- if (skb->nf_bridge &&
- skb->nf_bridge->mask & (BRNF_BRIDGED | BRNF_BRIDGED_DNAT))
- return nf_bridge_copy_header(skb);
- return 0;
-}
-
static inline unsigned int nf_bridge_encap_header_len(const struct sk_buff *skb)
{
switch (skb->protocol) {
@@ -46,6 +36,44 @@ static inline unsigned int nf_bridge_encap_header_len(const struct sk_buff *skb)
}
}
+static inline void nf_bridge_update_protocol(struct sk_buff *skb)
+{
+ if (skb->nf_bridge->mask & BRNF_8021Q)
+ skb->protocol = htons(ETH_P_8021Q);
+ else if (skb->nf_bridge->mask & BRNF_PPPoE)
+ skb->protocol = htons(ETH_P_PPP_SES);
+}
+
+/* Fill in the header for fragmented IP packets handled by
+ * the IPv4 connection tracking code.
+ *
+ * Only used in br_forward.c
+ */
+static inline int nf_bridge_copy_header(struct sk_buff *skb)
+{
+ int err;
+ unsigned int header_size;
+
+ nf_bridge_update_protocol(skb);
+ header_size = ETH_HLEN + nf_bridge_encap_header_len(skb);
+ err = skb_cow_head(skb, header_size);
+ if (err)
+ return err;
+
+ skb_copy_to_linear_data_offset(skb, -header_size,
+ skb->nf_bridge->data, header_size);
+ __skb_push(skb, nf_bridge_encap_header_len(skb));
+ return 0;
+}
+
+static inline int nf_bridge_maybe_copy_header(struct sk_buff *skb)
+{
+ if (skb->nf_bridge &&
+ skb->nf_bridge->mask & (BRNF_BRIDGED | BRNF_BRIDGED_DNAT))
+ return nf_bridge_copy_header(skb);
+ return 0;
+}
+
static inline unsigned int nf_bridge_mtu_reduction(const struct sk_buff *skb)
{
if (unlikely(skb->nf_bridge->mask & BRNF_PPPoE))
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 07c9fdd..c4ff43f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -156,7 +156,7 @@ struct nf_conntrack {
};
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
struct nf_bridge_info {
atomic_t use;
unsigned int mask;
@@ -560,7 +560,7 @@ struct sk_buff {
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
struct nf_conntrack *nfct;
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
struct nf_bridge_info *nf_bridge;
#endif
@@ -2977,7 +2977,7 @@ static inline void nf_conntrack_get(struct nf_conntrack *nfct)
atomic_inc(&nfct->use);
}
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
static inline void nf_bridge_put(struct nf_bridge_info *nf_bridge)
{
if (nf_bridge && atomic_dec_and_test(&nf_bridge->use))
@@ -2995,7 +2995,7 @@ static inline void nf_reset(struct sk_buff *skb)
nf_conntrack_put(skb->nfct);
skb->nfct = NULL;
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
nf_bridge_put(skb->nf_bridge);
skb->nf_bridge = NULL;
#endif
@@ -3016,7 +3016,7 @@ static inline void __nf_copy(struct sk_buff *dst, const struct sk_buff *src)
nf_conntrack_get(src->nfct);
dst->nfctinfo = src->nfctinfo;
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
dst->nf_bridge = src->nf_bridge;
nf_bridge_get(src->nf_bridge);
#endif
@@ -3030,7 +3030,7 @@ static inline void nf_copy(struct sk_buff *dst, const struct sk_buff *src)
#if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
nf_conntrack_put(dst->nfct);
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
nf_bridge_put(dst->nf_bridge);
#endif
__nf_copy(dst, src);
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 47f4254..f60558d 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -373,7 +373,7 @@ static inline int neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
return 0;
}
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
static inline int neigh_hh_bridge(struct hh_cache *hh, struct sk_buff *skb)
{
unsigned int seq, hh_alen;
diff --git a/include/net/netfilter/ipv4/nf_reject.h b/include/net/netfilter/ipv4/nf_reject.h
index 931fbf8..f713b5a 100644
--- a/include/net/netfilter/ipv4/nf_reject.h
+++ b/include/net/netfilter/ipv4/nf_reject.h
@@ -98,7 +98,7 @@ static void nf_send_reset(struct sk_buff *oldskb, int hook)
nf_ct_attach(nskb, oldskb);
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
/* If we use ip_local_out for bridged traffic, the MAC source on
* the RST will be ours, instead of the destination's. This confuses
* some routers/firewalls, and they drop the packet. So we need to
diff --git a/include/net/netfilter/ipv6/nf_reject.h b/include/net/netfilter/ipv6/nf_reject.h
index 710d17e..7a10cfc 100644
--- a/include/net/netfilter/ipv6/nf_reject.h
+++ b/include/net/netfilter/ipv6/nf_reject.h
@@ -147,7 +147,7 @@ static void nf_send_reset6(struct net *net, struct sk_buff *oldskb, int hook)
nf_ct_attach(nskb, oldskb);
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
/* If we use ip6_local_out for bridged traffic, the MAC source on
* the RST will be ours, instead of the destination's. This confuses
* some routers/firewalls, and they drop the packet. So we need to
diff --git a/net/Kconfig b/net/Kconfig
index 4051fdf..f545b34 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -176,7 +176,7 @@ config NETFILTER_ADVANCED
If unsure, say Y.
config BRIDGE_NETFILTER
- bool "Bridged IP/ARP packets filtering"
+ tristate "Bridged IP/ARP packets filtering"
depends on BRIDGE && NETFILTER && INET
depends on NETFILTER_ADVANCED
default y
diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index 8590b94..5e3eac5 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -6,11 +6,12 @@ obj-$(CONFIG_BRIDGE) += bridge.o
bridge-y := br.o br_device.o br_fdb.o br_forward.o br_if.o br_input.o \
br_ioctl.o br_stp.o br_stp_bpdu.o \
- br_stp_if.o br_stp_timer.o br_netlink.o
+ br_stp_if.o br_stp_timer.o br_netlink.o \
+ br_nf_core.o
bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
-bridge-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
+obj-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o
diff --git a/net/bridge/br.c b/net/bridge/br.c
index 1a755a1..fa05345 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -143,6 +143,26 @@ static const struct stp_proto br_stp_proto = {
.rcv = br_stp_rcv,
};
+static void __br_nf_request_module(struct work_struct *w)
+{
+ kfree(w);
+ request_module("br_netfilter");
+}
+
+static int br_netfilter_request_module(void)
+{
+ struct work_struct *w;
+
+ w = kzalloc(sizeof(*w), GFP_KERNEL);
+ if (!w)
+ return -ENOMEM;
+
+ INIT_WORK(w, __br_nf_request_module);
+ schedule_work(w);
+
+ return 0;
+}
+
static int __init br_init(void)
{
int err;
@@ -161,7 +181,7 @@ static int __init br_init(void)
if (err)
goto err_out1;
- err = br_netfilter_init();
+ err = br_nf_core_init();
if (err)
goto err_out2;
@@ -178,12 +198,27 @@ static int __init br_init(void)
#if IS_ENABLED(CONFIG_ATM_LANE)
br_fdb_test_addr_hook = br_fdb_test_addr;
#endif
+ err = br_netfilter_request_module();
+ if (err < 0)
+ goto err_out5;
+
+ pr_info("bridge: automatic filtering via arp/ip/ip6tables is "
+ "deprecated and it will not be enabled by the bridge "
+ "core anymore soon. Update your scripts to load "
+ "br_netfilter if you need this. ");
return 0;
+
+err_out5:
+#if IS_ENABLED(CONFIG_ATM_LANE)
+ br_fdb_test_addr_hook = NULL;
+#endif
+ brioctl_set(NULL);
+ br_netlink_fini();
err_out4:
unregister_netdevice_notifier(&br_device_notifier);
err_out3:
- br_netfilter_fini();
+ br_nf_core_fini();
err_out2:
unregister_pernet_subsys(&br_net_ops);
err_out1:
@@ -196,20 +231,17 @@ err_out:
static void __exit br_deinit(void)
{
stp_proto_unregister(&br_stp_proto);
-
br_netlink_fini();
unregister_netdevice_notifier(&br_device_notifier);
brioctl_set(NULL);
-
unregister_pernet_subsys(&br_net_ops);
rcu_barrier(); /* Wait for completion of call_rcu()'s */
- br_netfilter_fini();
+ br_nf_core_fini();
#if IS_ENABLED(CONFIG_ATM_LANE)
br_fdb_test_addr_hook = NULL;
#endif
-
br_fdb_fini();
}
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 568cccd..659cac1 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -36,7 +36,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
u16 vid = 0;
rcu_read_lock();
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
if (skb->nf_bridge && (skb->nf_bridge->mask & BRNF_BRIDGED_DNAT)) {
br_nf_pre_routing_finish_bridge_slow(skb);
rcu_read_unlock();
@@ -167,7 +167,7 @@ static int br_change_mtu(struct net_device *dev, int new_mtu)
dev->mtu = new_mtu;
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
/* remember the MTU in the rtable for PMTU */
dst_metric_set(&br->fake_rtable.dst, RTAX_MTU, new_mtu);
#endif
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 056b67b..992ec49 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -49,6 +49,7 @@ int br_dev_queue_push_xmit(struct sk_buff *skb)
return 0;
}
+EXPORT_SYMBOL_GPL(br_dev_queue_push_xmit);
int br_forward_finish(struct sk_buff *skb)
{
@@ -56,6 +57,7 @@ int br_forward_finish(struct sk_buff *skb)
br_dev_queue_push_xmit);
}
+EXPORT_SYMBOL_GPL(br_forward_finish);
static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
{
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 366c436..6fd5522 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -140,6 +140,7 @@ drop:
kfree_skb(skb);
goto out;
}
+EXPORT_SYMBOL_GPL(br_handle_frame_finish);
/* note: already called with rcu_read_lock */
static int br_handle_local_finish(struct sk_buff *skb)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index a615264..97e4393 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -111,66 +111,6 @@ static inline __be16 pppoe_proto(const struct sk_buff *skb)
pppoe_proto(skb) == htons(PPP_IPV6) && \
brnf_filter_pppoe_tagged)
-static void fake_update_pmtu(struct dst_entry *dst, struct sock *sk,
- struct sk_buff *skb, u32 mtu)
-{
-}
-
-static void fake_redirect(struct dst_entry *dst, struct sock *sk,
- struct sk_buff *skb)
-{
-}
-
-static u32 *fake_cow_metrics(struct dst_entry *dst, unsigned long old)
-{
- return NULL;
-}
-
-static struct neighbour *fake_neigh_lookup(const struct dst_entry *dst,
- struct sk_buff *skb,
- const void *daddr)
-{
- return NULL;
-}
-
-static unsigned int fake_mtu(const struct dst_entry *dst)
-{
- return dst->dev->mtu;
-}
-
-static struct dst_ops fake_dst_ops = {
- .family = AF_INET,
- .protocol = cpu_to_be16(ETH_P_IP),
- .update_pmtu = fake_update_pmtu,
- .redirect = fake_redirect,
- .cow_metrics = fake_cow_metrics,
- .neigh_lookup = fake_neigh_lookup,
- .mtu = fake_mtu,
-};
-
-/*
- * Initialize bogus route table used to keep netfilter happy.
- * Currently, we fill in the PMTU entry because netfilter
- * refragmentation needs it, and the rt_flags entry because
- * ipt_REJECT needs it. Future netfilter modules might
- * require us to fill additional fields.
- */
-static const u32 br_dst_default_metrics[RTAX_MAX] = {
- [RTAX_MTU - 1] = 1500,
-};
-
-void br_netfilter_rtable_init(struct net_bridge *br)
-{
- struct rtable *rt = &br->fake_rtable;
-
- atomic_set(&rt->dst.__refcnt, 1);
- rt->dst.dev = br->dev;
- rt->dst.path = &rt->dst;
- dst_init_metrics(&rt->dst, br_dst_default_metrics, true);
- rt->dst.flags = DST_NOXFRM | DST_FAKE_RTABLE;
- rt->dst.ops = &fake_dst_ops;
-}
-
static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
{
struct net_bridge_port *port;
@@ -245,14 +185,6 @@ static inline void nf_bridge_save_header(struct sk_buff *skb)
skb->nf_bridge->data, header_size);
}
-static inline void nf_bridge_update_protocol(struct sk_buff *skb)
-{
- if (skb->nf_bridge->mask & BRNF_8021Q)
- skb->protocol = htons(ETH_P_8021Q);
- else if (skb->nf_bridge->mask & BRNF_PPPoE)
- skb->protocol = htons(ETH_P_PPP_SES);
-}
-
/* When handing a packet over to the IP layer
* check whether we have a skb that is in the
* expected format
@@ -320,26 +252,6 @@ drop:
return -1;
}
-/* Fill in the header for fragmented IP packets handled by
- * the IPv4 connection tracking code.
- */
-int nf_bridge_copy_header(struct sk_buff *skb)
-{
- int err;
- unsigned int header_size;
-
- nf_bridge_update_protocol(skb);
- header_size = ETH_HLEN + nf_bridge_encap_header_len(skb);
- err = skb_cow_head(skb, header_size);
- if (err)
- return err;
-
- skb_copy_to_linear_data_offset(skb, -header_size,
- skb->nf_bridge->data, header_size);
- __skb_push(skb, nf_bridge_encap_header_len(skb));
- return 0;
-}
-
/* PF_BRIDGE/PRE_ROUTING *********************************************/
/* Undo the changes made for ip6tables PREROUTING and continue the
* bridge PRE_ROUTING hook. */
@@ -1059,38 +971,42 @@ static struct ctl_table brnf_table[] = {
};
#endif
-int __init br_netfilter_init(void)
+static int __init br_netfilter_init(void)
{
int ret;
- ret = dst_entries_init(&fake_dst_ops);
+ ret = nf_register_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
if (ret < 0)
return ret;
- ret = nf_register_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
- if (ret < 0) {
- dst_entries_destroy(&fake_dst_ops);
- return ret;
- }
#ifdef CONFIG_SYSCTL
brnf_sysctl_header = register_net_sysctl(&init_net, "net/bridge", brnf_table);
if (brnf_sysctl_header == NULL) {
printk(KERN_WARNING
"br_netfilter: can't register to sysctl.\n");
- nf_unregister_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
- dst_entries_destroy(&fake_dst_ops);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err1;
}
#endif
printk(KERN_NOTICE "Bridge firewalling registered\n");
return 0;
+err1:
+ nf_unregister_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
+ return ret;
}
-void br_netfilter_fini(void)
+static void __exit br_netfilter_fini(void)
{
nf_unregister_hooks(br_nf_ops, ARRAY_SIZE(br_nf_ops));
#ifdef CONFIG_SYSCTL
unregister_net_sysctl_table(brnf_sysctl_header);
#endif
- dst_entries_destroy(&fake_dst_ops);
}
+
+module_init(br_netfilter_init);
+module_exit(br_netfilter_fini);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Lennert Buytenhek <buytenh@gnu.org>");
+MODULE_AUTHOR("Bart De Schuymer <bdschuym@pandora.be>");
+MODULE_DESCRIPTION("Linux ethernet netfilter firewall bridge");
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 90a91e1..0fa66b8 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -602,7 +602,7 @@ out_af:
return err;
}
-void __exit br_netlink_fini(void)
+void br_netlink_fini(void)
{
br_mdb_uninit();
rtnl_af_unregister(&br_af_ops);
diff --git a/net/bridge/br_nf_core.c b/net/bridge/br_nf_core.c
new file mode 100644
index 0000000..387cb3b
--- /dev/null
+++ b/net/bridge/br_nf_core.c
@@ -0,0 +1,96 @@
+/*
+ * Handle firewalling core
+ * Linux ethernet bridge
+ *
+ * Authors:
+ * Lennert Buytenhek <buytenh@gnu.org>
+ * Bart De Schuymer <bdschuym@pandora.be>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Lennert dedicates this file to Kerstin Wurdinger.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/in_route.h>
+#include <linux/inetdevice.h>
+#include <net/route.h>
+
+#include "br_private.h"
+#ifdef CONFIG_SYSCTL
+#include <linux/sysctl.h>
+#endif
+
+static void fake_update_pmtu(struct dst_entry *dst, struct sock *sk,
+ struct sk_buff *skb, u32 mtu)
+{
+}
+
+static void fake_redirect(struct dst_entry *dst, struct sock *sk,
+ struct sk_buff *skb)
+{
+}
+
+static u32 *fake_cow_metrics(struct dst_entry *dst, unsigned long old)
+{
+ return NULL;
+}
+
+static struct neighbour *fake_neigh_lookup(const struct dst_entry *dst,
+ struct sk_buff *skb,
+ const void *daddr)
+{
+ return NULL;
+}
+
+static unsigned int fake_mtu(const struct dst_entry *dst)
+{
+ return dst->dev->mtu;
+}
+
+static struct dst_ops fake_dst_ops = {
+ .family = AF_INET,
+ .protocol = cpu_to_be16(ETH_P_IP),
+ .update_pmtu = fake_update_pmtu,
+ .redirect = fake_redirect,
+ .cow_metrics = fake_cow_metrics,
+ .neigh_lookup = fake_neigh_lookup,
+ .mtu = fake_mtu,
+};
+
+/*
+ * Initialize bogus route table used to keep netfilter happy.
+ * Currently, we fill in the PMTU entry because netfilter
+ * refragmentation needs it, and the rt_flags entry because
+ * ipt_REJECT needs it. Future netfilter modules might
+ * require us to fill additional fields.
+ */
+static const u32 br_dst_default_metrics[RTAX_MAX] = {
+ [RTAX_MTU - 1] = 1500,
+};
+
+void br_netfilter_rtable_init(struct net_bridge *br)
+{
+ struct rtable *rt = &br->fake_rtable;
+
+ atomic_set(&rt->dst.__refcnt, 1);
+ rt->dst.dev = br->dev;
+ rt->dst.path = &rt->dst;
+ dst_init_metrics(&rt->dst, br_dst_default_metrics, true);
+ rt->dst.flags = DST_NOXFRM | DST_FAKE_RTABLE;
+ rt->dst.ops = &fake_dst_ops;
+}
+
+int __init br_nf_core_init(void)
+{
+ return dst_entries_init(&fake_dst_ops);
+}
+
+void br_nf_core_fini(void)
+{
+ dst_entries_destroy(&fake_dst_ops);
+}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 62a7fa2..d304d75 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -221,7 +221,7 @@ struct net_bridge
struct pcpu_sw_netstats __percpu *stats;
spinlock_t hash_lock;
struct hlist_head hash[BR_HASH_SIZE];
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
struct rtable fake_rtable;
bool nf_call_iptables;
bool nf_call_ip6tables;
@@ -751,13 +751,13 @@ static inline int br_vlan_enabled(struct net_bridge *br)
#endif
/* br_netfilter.c */
-#ifdef CONFIG_BRIDGE_NETFILTER
-int br_netfilter_init(void);
-void br_netfilter_fini(void);
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
+int br_nf_core_init(void);
+void br_nf_core_fini(void);
void br_netfilter_rtable_init(struct net_bridge *);
#else
-#define br_netfilter_init() (0)
-#define br_netfilter_fini() do { } while (0)
+static inline int br_nf_core_init(void) { return 0; }
+static inline void br_nf_core_fini(void) {}
#define br_netfilter_rtable_init(x)
#endif
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index c9e2572..cb431c6 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -629,7 +629,7 @@ static ssize_t multicast_startup_query_interval_store(
}
static DEVICE_ATTR_RW(multicast_startup_query_interval);
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
static ssize_t nf_call_iptables_show(
struct device *d, struct device_attribute *attr, char *buf)
{
@@ -763,7 +763,7 @@ static struct attribute *bridge_attrs[] = {
&dev_attr_multicast_query_response_interval.attr,
&dev_attr_multicast_startup_query_interval.attr,
#endif
-#ifdef CONFIG_BRIDGE_NETFILTER
+#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
&dev_attr_nf_call_iptables.attr,
&dev_attr_nf_call_ip6tables.attr,
&dev_attr_nf_call_arptables.attr,
--
1.7.10.4
^ permalink raw reply related
* Re: CPU scheduler to TXQ binding? (ixgbe vs. igb)
From: Eric Dumazet @ 2014-09-18 13:33 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexander Duyck, netdev@vger.kernel.org, Tom Herbert
In-Reply-To: <20140918085640.0815df6d@redhat.com>
On Thu, 2014-09-18 at 08:56 +0200, Jesper Dangaard Brouer wrote:
> After setting up XPS to CPU 1:1 binding, it works most of the time.
> Meaning, most of the traffic will go through the TXQ I've bound the
> process to, BUT some packets can still choose another TXQ (observed
> monitoring tc output and blqmon).
Note that for TCP, there are packets sent by the process doing the
send(), or packets sent by cpu doing TX completion (because of TSQ),
but also packets sent by ACK processing done in the reverse way.
As Alexander explained, if the ACK packets are delivered into another
CPU, then you might select another TX queue.
This is mostly prevented because of ooo_okay logic, meaning that a busy
bulk flow should stick into a single TX queue, no matter of XPS says.
A TCP_RR workload is free to chose whatever queue, because every packet
starting a RR block has the ooo_okay set (As prior data was delivered
and acknowledged by the opposite peer)
>
> Could this be related to the missing RPS setup?
No, for this to really work, you need hardware support, so that ACK
packets take the same RX queue than the sent packets.
>
> Can I get some hints setting up RPS?
>
Documentation/networking/scaling.txt is full of hints...
^ permalink raw reply
* Re: [PATCHv6 net-next 1/3] sunvnet: upgrade to VIO protocol version 1.6
From: David L Stevens @ 2014-09-18 13:21 UTC (permalink / raw)
To: Raghuram Kothakota; +Cc: David Miller, netdev
In-Reply-To: <F34B8038-D189-4DD7-BF98-26CC4BD11990@oracle.com>
On 09/18/2014 12:23 AM, Raghuram Kothakota wrote:
>>
>> + /* v1.6 and higher, ACK with desired, supported mode, or NACK */
>> + if (vio_version_after_eq(vio, 1, 6)) {
>> + if (!(pkt->options & VIO_TX_DRING))
>> + goto send_nack;
>> + pkt->options = VIO_TX_DRING;
>> + }
>> +
>
>
> I forget to send this comment in my previous email. The above function is
> common to all clients, that includes vdisk. The vdisk today doesn't use any
> version number above 1.2 or 1.3, so it probably won't impact it, but this
> special handling doesn't seem to belong in the common code.
Yes, this is updating the VIO protocol, which is used by both sunvnet and sunvdc.
If the vdisk uses a newer protocol version, it will need to do exactly what this
code does (and it already will, because this code is there).
Any protocol version differences in the vdisk-specific code will need to be part
of a vdisk version upgrade patch. The particular VIO version that is negotiated
is specified in the individual drivers and my patches update that for sunvnet only.
The sunvdc code is still negotiating and using 1.0 in the net-next kernel, so
none of these version checks change anything for the sunvdc driver.
+-DLS
^ permalink raw reply
* Re: Comments regarding patch about setting netns for wireless devices
From: John W. Linville @ 2014-09-18 13:13 UTC (permalink / raw)
To: Vadim Kochan
Cc: netdev@vger.kernel.org, linux-wireless, Emmanuel Grumbach,
Oliver Hartkopp, johannes
In-Reply-To: <CAMw6YJ+uhthY83_cBHE20FgqFWdvT5M12AbfgVC7qbyHegh78Q@mail.gmail.com>
On Thu, Sep 18, 2014 at 11:05:11AM +0300, Vadim Kochan wrote:
> Hi All,
>
> I'd like to get some feedback about the patch:
> https://patchwork.kernel.org/patch/4890451/
>
> The idea is that currently Linux wireless device is allowed to change
> the network namespace only by nl80211 API, so RTM_LINK API does
> not work. So I think that it should be possible to change netns
> through the RTM_LINK too as generic way to change netns,
> thats why I tried to solve this in the patch.
>
> Thanks,
I think Johannes is vacationing right now. I'm sure he will comment
when he returns.
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCHv6 net-next 2/3] sunvnet: allow admin to set sunvnet MTU
From: David L Stevens @ 2014-09-18 13:10 UTC (permalink / raw)
To: Raghuram Kothakota; +Cc: David Miller, netdev
In-Reply-To: <C383AC6E-F0A2-4510-8EC6-0EC606870249@oracle.com>
On 09/18/2014 12:19 AM, Raghuram Kothakota wrote:
>
> FYI, LDoms manager provides the maximum MTU of packets allowed for
> a vnet device in the "virtual-device" MD node corresponding to the vnet device.
> The virtual switch is expected to enforce that size for packets that go through
> it. Ideally we want a Guest OS to honor that MTU setting as well, which is probably
> not the case here. LDoms manager doesn't allow setting more than 16K today,
> I guess this code is ignoring this for Guest to Guest communication.
Yes, this is negotiated per LDC connection, and it uses the switch size when using
the switch port, but can use a full 64K linux-linux. Anything involving Solaris will
be limited to 16000, and any connections to Legacy linux will be limited to 1500 bytes.
If the port for a particular destination changes and the new port has a smaller MTU,
the packets will trigger an ICMP PMTUD notification and any active connections will
continue to work at the smaller MTU, by PMTUD design.
+-DLS
^ permalink raw reply
* Re: [PATCHv6 net-next 1/3] sunvnet: upgrade to VIO protocol version 1.6
From: David L Stevens @ 2014-09-18 13:03 UTC (permalink / raw)
To: Raghuram Kothakota; +Cc: David Miller, netdev
In-Reply-To: <2AB76E42-C12D-47C5-8476-0D0C611691A5@oracle.com>
On 09/18/2014 12:09 AM, Raghuram Kothakota wrote:
>> @@ -1048,8 +1116,8 @@ static int vnet_port_alloc_tx_bufs(struct vnet_port *port)
>> void *dring;
>>
>> for (i = 0; i < VNET_TX_RING_SIZE; i++) {
>> - void *buf = kzalloc(ETH_FRAME_LEN + 8, GFP_KERNEL);
>> - int map_len = (ETH_FRAME_LEN + 7) & ~7;
>> + void *buf = kzalloc(VNET_MAXPACKET + 8, GFP_KERNEL);
>
>
> This patch doesn't change the VNET_MAXPACKET to 64k, but the patch 2/3 changes
> it to 64k+. Allocating buffers of size VNET_MAXPACKET always can consume too much
> memory for every port/LDC, that would be more than 32MB. You may want to allocate
> buffers based on the mtu that is negotiated, so that this memory used only when
> such large packets are accepted by the peer.
I originally had code to dynamically allocate them after the MTU negotiation, but
that opens up a can of worms regarding stopping and freeing an active ring. I don't
believe the shutdown code addresses this adequately, either, and I think this is
worth addressing, but separately.
I convinced myself to do it this way because:
a) memory is cheap
b) I think most people will want to use large MTUs for performance; enough so
that perhaps the bring-up MTU should be 64K too
c) future (actually current) TSO/GSO work will want large buffers even if the MTU
is not changed
So, if this is actually too much memory, I was more inclined to reduce the ring
size rather than either add complicating code to handle active-ring reallocation
that would typically be run once per boot, or another alternative of adding
module parameters to specify the buffer size TSO/GSO will need 64K to perform
well, regardless of the device MTU.
+-DLS
^ permalink raw reply
* [PATCH] net: stmmac: fix stmmac_pci_probe failed when CONFIG_HAVE_CLK is selected
From: Kweh Hock Leong @ 2014-09-18 12:34 UTC (permalink / raw)
To: David S. Miller, Giuseppe Cavallaro, rayagond
Cc: Vince Bridgers, Chen-Yu Tsai, netdev, LKML, Ong Boon Leong,
Tobias Klausmann
From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>
When the CONFIG_HAVE_CLK is selected for the system, the stmmac_pci_probe
will fail with dmesg:
[ 2.167225] stmmaceth 0000:00:14.6: enabling device (0000 -> 0002)
[ 2.178267] stmmaceth 0000:00:14.6: enabling bus mastering
[ 2.178436] stmmaceth 0000:00:14.6: irq 24 for MSI/MSI-X
[ 2.178703] stmmaceth 0000:00:14.6: stmmac_dvr_probe: warning: cannot
get CSR clock
[ 2.186503] stmmac_pci_probe: main driver probe failed
[ 2.194003] stmmaceth 0000:00:14.6: disabling bus mastering
[ 2.196473] stmmaceth: probe of 0000:00:14.6 failed with error -2
This patch fix the issue by breaking the dependency to devm_clk_get()
as the CSR clock can be obtained at priv->plat->clk_csr from pci driver.
Reported-by: Tobias Klausmann <tobias.johannes.klausmann@mni.thm.de>
Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 08addd6..ea3859a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2714,10 +2714,15 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,
priv->stmmac_clk = devm_clk_get(priv->device, STMMAC_RESOURCE_NAME);
if (IS_ERR(priv->stmmac_clk)) {
- dev_warn(priv->device, "%s: warning: cannot get CSR clock\n",
- __func__);
- ret = PTR_ERR(priv->stmmac_clk);
- goto error_clk_get;
+ if (!priv->plat->clk_csr) {
+ dev_warn(priv->device,
+ "%s: warning: cannot get CSR clock\n",
+ __func__);
+ ret = PTR_ERR(priv->stmmac_clk);
+ goto error_clk_get;
+ } else {
+ priv->stmmac_clk = NULL;
+ }
}
clk_prepare_enable(priv->stmmac_clk);
--
1.7.9.5
^ permalink raw reply related
* [net-next 10/11] ixgbe: add warnings for other disabled features without MSI-X support
From: Jeff Kirsher @ 2014-09-18 12:27 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
When we can't get MSI-X vectors, we disable a few features which require
MSI-X vectors. Print warnings just like we do when disabling DCB.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 0ec22d8..ce40c77 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -1114,13 +1114,16 @@ static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
adapter->temp_dcb_cfg.pfc_mode_enable = false;
adapter->dcb_cfg.pfc_mode_enable = false;
}
+
adapter->dcb_cfg.num_tcs.pg_tcs = 1;
adapter->dcb_cfg.num_tcs.pfc_tcs = 1;
- /* disable SR-IOV */
+ /* Disable SR-IOV support */
+ e_dev_warn("Disabling SR-IOV support\n");
ixgbe_disable_sriov(adapter);
- /* disable RSS */
+ /* Disable RSS */
+ e_dev_warn("Disabling RSS support\n");
adapter->ring_feature[RING_F_RSS].limit = 1;
/* recalculate number of queues now that many features have been
--
1.9.3
^ permalink raw reply related
* [net-next 11/11] ixgbe: remove IXGBE_FLAG_MSI(X)_CAPABLE flags
From: Jeff Kirsher @ 2014-09-18 12:27 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
They were not used, and we don't need them, so we shouldn't bother with
keeping values in the flags field that could be misleading.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 75a56fa..673d820 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -611,9 +611,7 @@ struct ixgbe_adapter {
* thus the additional *_CAPABLE flags.
*/
u32 flags;
-#define IXGBE_FLAG_MSI_CAPABLE (u32)(1 << 0)
#define IXGBE_FLAG_MSI_ENABLED (u32)(1 << 1)
-#define IXGBE_FLAG_MSIX_CAPABLE (u32)(1 << 2)
#define IXGBE_FLAG_MSIX_ENABLED (u32)(1 << 3)
#define IXGBE_FLAG_RX_1BUF_CAPABLE (u32)(1 << 4)
#define IXGBE_FLAG_RX_PS_CAPABLE (u32)(1 << 5)
--
1.9.3
^ permalink raw reply related
* [net-next 09/11] ixgbe: use e_dev_warn instead of netif_printk
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
Again, we should not be directly using netif_printk, as we have our own
error print routines that we generate. In addition, instead of using an
early return we can just use the else block of this one line if
statement.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 969b979..0ec22d8 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -1130,13 +1130,11 @@ static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
adapter->num_q_vectors = 1;
err = pci_enable_msi(adapter->pdev);
- if (err) {
- netif_printk(adapter, hw, KERN_DEBUG, adapter->netdev,
- "Unable to allocate MSI interrupt, falling back to legacy. Error: %d\n",
- err);
- return;
- }
- adapter->flags |= IXGBE_FLAG_MSI_ENABLED;
+ if (err)
+ e_dev_warn("Failed to allocate MSI interrupt, falling back to legacy. Error: %d\n",
+ err);
+ else
+ adapter->flags |= IXGBE_FLAG_MSI_ENABLED;
}
/**
--
1.9.3
^ permalink raw reply related
* [net-next 08/11] ixgbe: use e_dev_warn instead of e_err for displaying warning
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
In this case, disabling DCB is not an error. We can still function, but
we just have to let the user know. In addition, since we call this
during probe before allocating our netdevice structure, we should use
e_dev_warn instead of e_warn.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 39df14f..969b979 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -1102,9 +1102,9 @@ static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
* capability.
*/
- /* disable DCB if number of TCs exceeds 1 */
+ /* Disable DCB unless we only have a single traffic class */
if (netdev_get_num_tc(adapter->netdev) > 1) {
- e_err(probe, "num TCs exceeds number of queues - disabling DCB\n");
+ e_dev_warn("Number of DCB TCs exceeds number of available queues. Disabling DCB support.\n");
netdev_reset_tc(adapter->netdev);
if (adapter->hw.mac.type == ixgbe_mac_82598EB)
--
1.9.3
^ permalink raw reply related
* [net-next 05/11] ixgbe: return integer from ixgbe_acquire_msix_vectors
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
Similar to how ixgbevf handles acquiring MSI-X vectors, we can return an
error code instead of relying on the flag being set. This makes it more
clear that we have failed to setup MSI-X mode, and also will make it
easier to consolidate MSI-X related code all into the single function.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 32 ++++++++++++++++------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 5d085d5..2a38894 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -696,8 +696,8 @@ static void ixgbe_set_num_queues(struct ixgbe_adapter *adapter)
ixgbe_set_rss_queues(adapter);
}
-static void ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
- int vectors)
+static int ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
+ int vectors)
{
int vector_threshold;
@@ -726,16 +726,22 @@ static void ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
adapter->flags &= ~IXGBE_FLAG_MSIX_ENABLED;
kfree(adapter->msix_entries);
adapter->msix_entries = NULL;
- } else {
- adapter->flags |= IXGBE_FLAG_MSIX_ENABLED; /* Woot! */
- /*
- * Adjust for only the vectors we'll use, which is minimum
- * of max_msix_q_vectors + NON_Q_VECTORS, or the number of
- * vectors we were allocated.
- */
- vectors -= NON_Q_VECTORS;
- adapter->num_q_vectors = min(vectors, adapter->max_q_vectors);
+
+ return vectors;
}
+
+ /* we successfully allocated some number of vectors within our
+ * requested range.
+ */
+ adapter->flags |= IXGBE_FLAG_MSIX_ENABLED;
+
+ /* Adjust for only the vectors we'll use, which is minimum
+ * of max_q_vectors, or the number of vectors we were allocated.
+ */
+ vectors -= NON_Q_VECTORS;
+ adapter->num_q_vectors = min_t(int, vectors, adapter->max_q_vectors);
+
+ return 0;
}
static void ixgbe_add_ring(struct ixgbe_ring *ring,
@@ -1085,9 +1091,7 @@ static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
for (vector = 0; vector < v_budget; vector++)
adapter->msix_entries[vector].entry = vector;
- ixgbe_acquire_msix_vectors(adapter, v_budget);
-
- if (adapter->flags & IXGBE_FLAG_MSIX_ENABLED)
+ if (!ixgbe_acquire_msix_vectors(adapter, v_budget))
return;
}
--
1.9.3
^ permalink raw reply related
* [net-next 06/11] ixgbe: move msix_entries allocation into ixgbe_acquire_msix_vectors
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
We already have to kfree this value if we fail, and this is only part of
MSI-X mode, so we should simply allocate the value where we need it.
This is cleaner, and makes it a lot more obvious why we are freeing it
inside of ixgbe_acquire_msix_vectors.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 2a38894..f504faf 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -699,7 +699,7 @@ static void ixgbe_set_num_queues(struct ixgbe_adapter *adapter)
static int ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
int vectors)
{
- int vector_threshold;
+ int i, vector_threshold;
/* We'll want at least 2 (vector_threshold):
* 1) TxQ[0] + RxQ[0] handler
@@ -713,6 +713,15 @@ static int ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
* Right now, we simply care about how many we'll get; we'll
* set them up later while requesting irq's.
*/
+ adapter->msix_entries = kcalloc(vectors,
+ sizeof(struct msix_entry),
+ GFP_KERNEL);
+ if (!adapter->msix_entries)
+ return -ENOMEM;
+
+ for (i = 0; i < vectors; i++)
+ adapter->msix_entries[i].entry = i;
+
vectors = pci_enable_msix_range(adapter->pdev, adapter->msix_entries,
vector_threshold, vectors);
@@ -1061,7 +1070,7 @@ static void ixgbe_reset_interrupt_capability(struct ixgbe_adapter *adapter)
static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
- int vector, v_budget, err;
+ int v_budget, err;
/*
* It's easy to be greedy for MSI-X vectors, but it really
@@ -1085,15 +1094,8 @@ static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
/* A failure in MSI-X entry allocation isn't fatal, but it does
* mean we disable MSI-X capabilities of the adapter. */
- adapter->msix_entries = kcalloc(v_budget,
- sizeof(struct msix_entry), GFP_KERNEL);
- if (adapter->msix_entries) {
- for (vector = 0; vector < v_budget; vector++)
- adapter->msix_entries[vector].entry = vector;
-
- if (!ixgbe_acquire_msix_vectors(adapter, v_budget))
- return;
- }
+ if (!ixgbe_acquire_msix_vectors(adapter, v_budget))
+ return;
/* At this point, we do not have MSI-X capabilities. We need to
* reconfigure or disable various features which require MSI-X
--
1.9.3
^ permalink raw reply related
* [net-next 07/11] ixgbe: determine vector count inside ixgbe_acquire_msix_vectors
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
Our calculated v_budget doesn't matter except if we allocate MSI-X
vectors. We shouldn't need to calculate this outside of the function, so
don't. Instead, only calculate it once we attempt to acquire MSI-X
vectors. This helps collocate all of the MSI-X vector code together.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 74 ++++++++++++++--------------
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index f504faf..39df14f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -696,23 +696,45 @@ static void ixgbe_set_num_queues(struct ixgbe_adapter *adapter)
ixgbe_set_rss_queues(adapter);
}
-static int ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
- int vectors)
+/**
+ * ixgbe_acquire_msix_vectors - acquire MSI-X vectors
+ * @adapter: board private structure
+ *
+ * Attempts to acquire a suitable range of MSI-X vector interrupts. Will
+ * return a negative error code if unable to acquire MSI-X vectors for any
+ * reason.
+ */
+static int ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter)
{
- int i, vector_threshold;
+ struct ixgbe_hw *hw = &adapter->hw;
+ int i, vectors, vector_threshold;
+
+ /* We start by asking for one vector per queue pair */
+ vectors = max(adapter->num_rx_queues, adapter->num_tx_queues);
- /* We'll want at least 2 (vector_threshold):
- * 1) TxQ[0] + RxQ[0] handler
- * 2) Other (Link Status Change, etc.)
+ /* It is easy to be greedy for MSI-X vectors. However, it really
+ * doesn't do much good if we have a lot more vectors than CPUs. We'll
+ * be somewhat conservative and only ask for (roughly) the same number
+ * of vectors as there are CPUs.
*/
- vector_threshold = MIN_MSIX_COUNT;
+ vectors = min_t(int, vectors, num_online_cpus());
- /*
- * The more we get, the more we will assign to Tx/Rx Cleanup
- * for the separate queues...where Rx Cleanup >= Tx Cleanup.
- * Right now, we simply care about how many we'll get; we'll
- * set them up later while requesting irq's.
+ /* Some vectors are necessary for non-queue interrupts */
+ vectors += NON_Q_VECTORS;
+
+ /* Hardware can only support a maximum of hw.mac->max_msix_vectors.
+ * With features such as RSS and VMDq, we can easily surpass the
+ * number of Rx and Tx descriptor queues supported by our device.
+ * Thus, we cap the maximum in the rare cases where the CPU count also
+ * exceeds our vector limit
+ */
+ vectors = min_t(int, vectors, hw->mac.max_msix_vectors);
+
+ /* We want a minimum of two MSI-X vectors for (1) a TxQ[0] + RxQ[0]
+ * handler, and (2) an Other (Link Status Change, etc.) handler.
*/
+ vector_threshold = MIN_MSIX_COUNT;
+
adapter->msix_entries = kcalloc(vectors,
sizeof(struct msix_entry),
GFP_KERNEL);
@@ -1069,32 +1091,10 @@ static void ixgbe_reset_interrupt_capability(struct ixgbe_adapter *adapter)
**/
static void ixgbe_set_interrupt_capability(struct ixgbe_adapter *adapter)
{
- struct ixgbe_hw *hw = &adapter->hw;
- int v_budget, err;
-
- /*
- * It's easy to be greedy for MSI-X vectors, but it really
- * doesn't do us much good if we have a lot more vectors
- * than CPU's. So let's be conservative and only ask for
- * (roughly) the same number of vectors as there are CPU's.
- * The default is to use pairs of vectors.
- */
- v_budget = max(adapter->num_rx_queues, adapter->num_tx_queues);
- v_budget = min_t(int, v_budget, num_online_cpus());
- v_budget += NON_Q_VECTORS;
-
- /*
- * At the same time, hardware can only support a maximum of
- * hw.mac->max_msix_vectors vectors. With features
- * such as RSS and VMDq, we can easily surpass the number of Rx and Tx
- * descriptor queues supported by our device. Thus, we cap it off in
- * those rare cases where the cpu count also exceeds our vector limit.
- */
- v_budget = min_t(int, v_budget, hw->mac.max_msix_vectors);
+ int err;
- /* A failure in MSI-X entry allocation isn't fatal, but it does
- * mean we disable MSI-X capabilities of the adapter. */
- if (!ixgbe_acquire_msix_vectors(adapter, v_budget))
+ /* We will try to get MSI-X interrupts first */
+ if (!ixgbe_acquire_msix_vectors(adapter))
return;
/* At this point, we do not have MSI-X capabilities. We need to
--
1.9.3
^ permalink raw reply related
* [net-next 03/11] ixgbe: Do not schedule an uninitialized workqueue entry
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Mark Rustad, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mark Rustad <mark.d.rustad@intel.com>
If a hardware Tx timestamp is requested, an uninitialized
workqueue entry may be scheduled, especially on an 82598 adapter.
Add a check for a PTP clock to avoid that. Also only apply the
unlikely to the first term of the conditional. That will make the
rest of the checks be in the cold path.
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Acked-by: Matthew Vick <matthew.vick@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bcd697c..06ef5a3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7108,9 +7108,10 @@ netdev_tx_t ixgbe_xmit_frame_ring(struct sk_buff *skb,
tx_flags |= IXGBE_TX_FLAGS_SW_VLAN;
}
- if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
- !test_and_set_bit_lock(__IXGBE_PTP_TX_IN_PROGRESS,
- &adapter->state))) {
+ if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+ adapter->ptp_clock &&
+ !test_and_set_bit_lock(__IXGBE_PTP_TX_IN_PROGRESS,
+ &adapter->state)) {
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
tx_flags |= IXGBE_TX_FLAGS_TSTAMP;
--
1.9.3
^ permalink raw reply related
* [net-next 04/11] ixgbe: use e_dev_warn instead of netif_printk
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jacob Keller <jacob.e.keller@intel.com>
The netif_printk relies on our netdevice structure to be registered
already. We may call ixgbe_acquire_msix_vectors prior to registering our
netdevice, so we should not use the netdevice specific printk.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 7ecd99c..5d085d5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -717,12 +717,12 @@ static void ixgbe_acquire_msix_vectors(struct ixgbe_adapter *adapter,
vector_threshold, vectors);
if (vectors < 0) {
- /* Can't allocate enough MSI-X interrupts? Oh well.
- * This just means we'll go with either a single MSI
- * vector or fall back to legacy interrupts.
+ /* A negative count of allocated vectors indicates an error in
+ * acquiring within the specified range of MSI-X vectors
*/
- netif_printk(adapter, hw, KERN_DEBUG, adapter->netdev,
- "Unable to allocate MSI-X interrupts\n");
+ e_dev_warn("Failed to allocate MSI-X interrupts. Err: %d\n",
+ vectors);
+
adapter->flags &= ~IXGBE_FLAG_MSIX_ENABLED;
kfree(adapter->msix_entries);
adapter->msix_entries = NULL;
--
1.9.3
^ permalink raw reply related
* [net-next 02/11] ixgbe: remove useless bd_number from adapter struct
From: Jeff Kirsher @ 2014-09-18 12:26 UTC (permalink / raw)
To: davem; +Cc: Ethan Zhao, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1411043221-7163-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Ethan Zhao <ethan.zhao@oracle.com>
Because bd_number is not useful anymore, so remove it from adapter struct, or
if keep it, we have to fix the boards driven counter bug in ixgbe_remove() and
ixgbe_probe() only for trivial debug purpose -- other output is enough.
Signed-off-by: Ethan Zhao <ethan.zhao@oracle.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 --
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 06744f8..75a56fa 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -728,8 +728,6 @@ struct ixgbe_adapter {
u8 __iomem *io_addr; /* Mainly for iounmap use */
u32 wol;
- u16 bd_number;
-
u16 eeprom_verh;
u16 eeprom_verl;
u16 eeprom_cap;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 6a12bb1..bcd697c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7981,7 +7981,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
struct ixgbe_adapter *adapter = NULL;
struct ixgbe_hw *hw;
const struct ixgbe_info *ii = ixgbe_info_tbl[ent->driver_data];
- static int cards_found;
int i, err, pci_using_dac, expected_gts;
unsigned int indices = MAX_TX_QUEUES;
u8 part_str[IXGBE_PBANUM_LENGTH];
@@ -8067,8 +8066,6 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
netdev->watchdog_timeo = 5 * HZ;
strlcpy(netdev->name, pci_name(pdev), sizeof(netdev->name));
- adapter->bd_number = cards_found;
-
/* Setup hw api */
memcpy(&hw->mac.ops, ii->mac_ops, sizeof(hw->mac.ops));
hw->mac.type = ii->mac;
@@ -8352,7 +8349,6 @@ skip_sriov:
ixgbe_add_sanmac_netdev(netdev);
e_dev_info("%s\n", ixgbe_default_device_descr);
- cards_found++;
#ifdef CONFIG_IXGBE_HWMON
if (ixgbe_sysfs_init(adapter))
--
1.9.3
^ permalink raw reply related
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