* Re: [PATCH] fs/select: add vmalloc fallback for select(2)
From: Eric Dumazet @ 2016-09-22 16:24 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Alexander Viro, linux-fsdevel, linux-kernel, linux-mm,
Michal Hocko, netdev
In-Reply-To: <20160922152831.24165-1-vbabka@suse.cz>
On Thu, 2016-09-22 at 17:28 +0200, Vlastimil Babka wrote:
> The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
> with the number of fds passed. We had a customer report page allocation
> failures of order-4 for this allocation. This is a costly order, so it might
> easily fail, as the VM expects such allocation to have a lower-order fallback.
>
> Such trivial fallback is vmalloc(), as the memory doesn't have to be
> physically contiguous. Also the allocation is temporary for the duration of the
> syscall, so it's unlikely to stress vmalloc too much.
>
> Note that the poll(2) syscall seems to use a linked list of order-0 pages, so
> it doesn't need this kind of fallback.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> fs/select.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/fs/select.c b/fs/select.c
> index 8ed9da50896a..8fe5bddbe99b 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -29,6 +29,7 @@
> #include <linux/sched/rt.h>
> #include <linux/freezer.h>
> #include <net/busy_poll.h>
> +#include <linux/vmalloc.h>
>
> #include <asm/uaccess.h>
>
> @@ -558,6 +559,7 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
> struct fdtable *fdt;
> /* Allocate small arguments on the stack to save memory and be faster */
> long stack_fds[SELECT_STACK_ALLOC/sizeof(long)];
> + unsigned long alloc_size;
>
> ret = -EINVAL;
> if (n < 0)
> @@ -580,10 +582,15 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
> bits = stack_fds;
> if (size > sizeof(stack_fds) / 6) {
> /* Not enough space in on-stack array; must use kmalloc */
> + alloc_size = 6 * size;
> ret = -ENOMEM;
> - bits = kmalloc(6 * size, GFP_KERNEL);
> - if (!bits)
> - goto out_nofds;
> + bits = kmalloc(alloc_size, GFP_KERNEL|__GFP_NOWARN);
> + if (!bits && alloc_size > PAGE_SIZE) {
> + bits = vmalloc(alloc_size);
> +
> + if (!bits)
> + goto out_nofds;
Test should happen if alloc_size <= PAGE_SIZE
> + }
if (!bits && alloc_size > PAGE_SIZE)
bits = vmalloc(alloc_size);
if (!bits)
goto out_nofds;
> }
> fds.in = bits;
> fds.out = bits + size;
> @@ -618,7 +625,7 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
>
> out:
> if (bits != stack_fds)
> - kfree(bits);
> + kvfree(bits);
> out_nofds:
> return ret;
> }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH net-next] Documentation: devicetree: revise ethernet device-tree binding about TRGMII
From: sean.wang @ 2016-09-22 16:16 UTC (permalink / raw)
To: davem, sergei.shtylyov, john
Cc: nbd, netdev, linux-kernel, linux-mediatek, devicetree, keyhaede,
objelf, Sean Wang
From: Sean Wang <sean.wang@mediatek.com>
fix typo in mediatek-net.txt and add phy-mode "trgmii" to ethernet.txt
Cc: devicetree@vger.kernel.org
Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
Documentation/devicetree/bindings/net/ethernet.txt | 4 ++--
Documentation/devicetree/bindings/net/mediatek-net.txt | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/ethernet.txt b/Documentation/devicetree/bindings/net/ethernet.txt
index 5d88f37..e1d7681 100644
--- a/Documentation/devicetree/bindings/net/ethernet.txt
+++ b/Documentation/devicetree/bindings/net/ethernet.txt
@@ -11,8 +11,8 @@ The following properties are common to the Ethernet controllers:
the maximum frame size (there's contradiction in ePAPR).
- phy-mode: string, operation mode of the PHY interface; supported values are
"mii", "gmii", "sgmii", "qsgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id",
- "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-facto
- standard property;
+ "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii", "trgmii"; this is now a
+ de-facto standard property;
- phy-connection-type: the same as "phy-mode" property but described in ePAPR;
- phy-handle: phandle, specifies a reference to a node representing a PHY
device; this property is described in ePAPR and so preferred;
diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
index bae848e..da05f8b 100644
--- a/Documentation/devicetree/bindings/net/mediatek-net.txt
+++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
@@ -34,7 +34,7 @@ Required properties:
- phy-handle: see ethernet.txt file in the same directory and
the phy-mode "trgmii" required being provided when reg
is equal to 0 and the MAC uses fixed-link to connect
- with inernal switch such as MT7530.
+ with internal switch such as MT7530.
Example:
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-22 16:14 UTC (permalink / raw)
To: Edward Cree
Cc: Eric Dumazet, netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
James Morris, Trond Myklebust, Alexander Duyck, Daniel Borkmann,
Eric Dumazet, Tom Herbert, Hannes Frederic Sowa,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <589839b3-5930-2527-b0a3-315be254a175-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
On Thu, 2016-09-22 at 16:21 +0100, Edward Cree wrote:
> On 22/09/16 11:33, Paolo Abeni wrote:
> > Hi Eric,
> >
> > On Wed, 2016-09-21 at 16:31 -0700, Eric Dumazet wrote:
> >> Also does inet_diag properly give the forward_alloc to user ?
> >>
> >> $ ss -mua
> >> State Recv-Q Send-Q Local Address:Port Peer Addres
> >> s:Port
> >> UNCONN 51584 0 *:52460 *:*
> >> skmem:(r51584,rb327680,t0,tb327680,f1664,w0,o0,bl0,d575)
> > Thank you very much for reviewing this!
> >
> > My bad, there is still a race which leads to temporary negative values
> > of fwd. I feel the fix is trivial but it needs some investigation.
> >
> >> Couldn't we instead use an union of an atomic_t and int for
> >> sk->sk_forward_alloc ?
> > That was our first attempt, but we had some issue on mem scheduling; if
> > we use:
> >
> > if (atomic_sub_return(size, &sk->sk_forward_alloc_atomic) < 0) {
> > // fwd alloc
> > }
> >
> > that leads to inescapable, temporary, negative value for
> > sk->sk_forward_alloc.
> >
> > Another option would be:
> >
> > again:
> > fwd = atomic_read(&sk->sk_forward_alloc_atomic);
> > if (fwd > size) {
> > if (atomic_cmpxchg(&sk->sk_forward_alloc_atomic, fwd, fwd - size) != fwd)
> > goto again;
> > } else
> > // fwd alloc
> >
> > which would be bad under high contention.
> Apologies if I'm misunderstanding the problem, but couldn't you have two
> atomic_t fields, 'internal' and 'external' forward_alloc. Then
> if (atomic_sub_return(size, &sk->sk_forward_alloc_internal) < 0) {
> atomic_sub(size, &sk->sk_forward_alloc);
> // fwd alloc
> } else {
> atomic_add(size, &sk->sk_forward_alloc_internal);
> }
> or something like that. Then sk->sk_forward_alloc never sees a negative
> value, and is always >= sk->sk_forward_alloc_internal. Of course places
> that go the other way would have to add to sk->sk_forward_alloc first,
> before adding to sk->sk_forward_alloc_internal, to maintain that invariant.
I think that the idea behind using atomic ops directly on
sk_forward_alloc is to avoid adding other fields to the udp_socket.
If we can add some fields to the udp_sock structure, the schema proposed
in this patch should fit better (modulo bugs ;-), always requiring a
single atomic operation at memory reclaiming time and at memory
allocation time.
Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH iproute2 net-next] tc: m_vlan: Add vlan modify action
From: Stephen Hemminger @ 2016-09-22 16:05 UTC (permalink / raw)
To: Shmulik Ladkani; +Cc: netdev, Jamal Hadi Salim, Jiri Pirko, Shmulik Ladkani
In-Reply-To: <1474536670-2495-1-git-send-email-shmulik.ladkani@gmail.com>
On Thu, 22 Sep 2016 12:31:10 +0300
Shmulik Ladkani <shmulik.ladkani@ravellosystems.com> wrote:
> +
> +static const char *action_name(int action)
> +{
> + static const char * const names[] = {
> + [TCA_VLAN_ACT_POP] = "pop",
> + [TCA_VLAN_ACT_PUSH] = "push",
> + [TCA_VLAN_ACT_MODIFY] = "modify",
> + };
> + return names[action];
> +}
> +
Why are you wrapping a simple array lookup in a function?
^ permalink raw reply
* [PATCH net-next] net_sched: sch_fq: account for schedule/timers drifts
From: Eric Dumazet @ 2016-09-22 15:58 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
It looks like the following patch can make FQ very precise, even in VM
or stressed hosts. It matters at high pacing rates.
We take into account the difference between the time that was programmed
when last packet was sent, and current time (a drift of tens of usecs is
often observed)
Add an EWMA of the unthrottle latency to help diagnostics.
This latency is the difference between current time and oldest packet in
delayed RB-tree. This accounts for the high resolution timer latency,
but can be different under stress, as fq_check_throttled() can be
opportunistically be called from a dequeue() called after an enqueue()
for a different flow.
Tested:
// Start a 10Gbit flow
$ netperf --google-pacing-rate 1250000000 -H lpaa24 -l 10000 -- -K bbr &
Before patch :
$ sar -n DEV 10 5 | grep eth0 | grep Average
Average: eth0 17106.04 756876.84 1102.75 1119049.02 0.00 0.00 0.52
After patch :
$ sar -n DEV 10 5 | grep eth0 | grep Average
Average: eth0 17867.00 800245.90 1151.77 1183172.12 0.00 0.00 0.52
A new iproute2 tc can output the 'unthrottle latency' :
$ tc -s qd sh dev eth0 | grep latency
0 gc, 0 highprio, 32490767 throttled, 2382 ns latency
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/uapi/linux/pkt_sched.h | 2 +-
net/sched/sch_fq.c | 21 ++++++++++++++++++---
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index f8e39dbaa781..df7451d35131 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -811,7 +811,7 @@ struct tc_fq_qd_stats {
__u32 flows;
__u32 inactive_flows;
__u32 throttled_flows;
- __u32 pad;
+ __u32 unthrottle_latency_ns;
};
/* Heavy-Hitter Filter */
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 5dd929cc1423..18e752439f6f 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -86,6 +86,7 @@ struct fq_sched_data {
struct rb_root delayed; /* for rate limited flows */
u64 time_next_delayed_flow;
+ unsigned long unthrottle_latency_ns;
struct fq_flow internal; /* for non classified or high prio packets */
u32 quantum;
@@ -408,11 +409,19 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch,
static void fq_check_throttled(struct fq_sched_data *q, u64 now)
{
+ unsigned long sample;
struct rb_node *p;
if (q->time_next_delayed_flow > now)
return;
+ /* Update unthrottle latency EWMA.
+ * This is cheap and can help diagnosing timer/latency problems.
+ */
+ sample = (unsigned long)(now - q->time_next_delayed_flow);
+ q->unthrottle_latency_ns -= q->unthrottle_latency_ns >> 3;
+ q->unthrottle_latency_ns += sample >> 3;
+
q->time_next_delayed_flow = ~0ULL;
while ((p = rb_first(&q->delayed)) != NULL) {
struct fq_flow *f = container_of(p, struct fq_flow, rate_node);
@@ -515,7 +524,12 @@ begin:
len = NSEC_PER_SEC;
q->stat_pkts_too_long++;
}
-
+ /* Account for schedule/timers drifts.
+ * f->time_next_packet was set when prior packet was sent,
+ * and current time (@now) can be too late by tens of us.
+ */
+ if (f->time_next_packet)
+ len -= min(len/2, now - f->time_next_packet);
f->time_next_packet = now + len;
}
out:
@@ -787,6 +801,7 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt)
q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch));
q->flow_refill_delay = msecs_to_jiffies(40);
q->flow_max_rate = ~0U;
+ q->time_next_delayed_flow = ~0ULL;
q->rate_enable = 1;
q->new_flows.first = NULL;
q->old_flows.first = NULL;
@@ -854,8 +869,8 @@ static int fq_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
st.flows = q->flows;
st.inactive_flows = q->inactive_flows;
st.throttled_flows = q->throttled_flows;
- st.pad = 0;
-
+ st.unthrottle_latency_ns = min_t(unsigned long,
+ q->unthrottle_latency_ns, ~0U);
sch_tree_unlock(sch);
return gnet_stats_copy_app(d, &st, sizeof(st));
^ permalink raw reply related
* Re: [PATCH v6 5/6] net: ipv4, ipv6: run cgroup eBPF egress programs
From: Daniel Mack @ 2016-09-22 15:53 UTC (permalink / raw)
To: Daniel Borkmann, Pablo Neira Ayuso, Thomas Graf
Cc: htejun-b10kYP2dOMg, ast-b10kYP2dOMg, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
kafai-b10kYP2dOMg, fw-HFFVJYpyMKqzQB+pC5nmwQ,
harald-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
sargun-GaZTRHToo+CzQB+pC5nmwQ, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <57E3F4F9.70300-FeC+5ew28dpmcu3hnIyYJQ@public.gmane.org>
On 09/22/2016 05:12 PM, Daniel Borkmann wrote:
> On 09/22/2016 02:05 PM, Pablo Neira Ayuso wrote:
>> Benefits are, rewording previous email:
>>
>> * You get access to all of the existing netfilter hooks in one go
>> to run bpf programs. No need for specific redundant hooks. This
>> provides raw access to the netfilter hook, you define the little
>> code that your hook runs before you bpf run invocation. So there
>> is *no need to bloat the stack with more hooks, we use what we
>> have.*
>
> But also this doesn't really address the fundamental underlying problem
> that is discussed here. nft doesn't even have cgroups v2 support, only
> xt_cgroups has it so far, but even if it would have it, then it's still
> a scalability issue that this model has over what is being proposed by
> Daniel, since you still need to test linearly wrt cgroups v2 membership,
> whereas in the set that is proposed it's integral part of cgroups and can
> be extended further, also for non-networking users to use this facility.
> Or would the idea be that the current netfilter hooks would be redone in
> a way that they are generic enough so that any other user could make use
> of it independent of netfilter?
Yes, that part I don't understand either.
Pablo, could you outline in more detail (in terms of syscalls, commands,
resulting nftables layout etc) how your proposed model would support
having per-cgroup byte and packet counters for both ingress and egress,
and filtering at least for ingress?
And how would that mitigate the race gaps you have been worried about,
between cgroup creation and filters taking effect for a task?
Thanks,
Daniel
^ permalink raw reply
* [PATCH] fs/select: add vmalloc fallback for select(2)
From: Vlastimil Babka @ 2016-09-22 15:28 UTC (permalink / raw)
To: Alexander Viro
Cc: linux-fsdevel, linux-kernel, linux-mm, Michal Hocko, netdev,
Vlastimil Babka
The select(2) syscall performs a kmalloc(size, GFP_KERNEL) where size grows
with the number of fds passed. We had a customer report page allocation
failures of order-4 for this allocation. This is a costly order, so it might
easily fail, as the VM expects such allocation to have a lower-order fallback.
Such trivial fallback is vmalloc(), as the memory doesn't have to be
physically contiguous. Also the allocation is temporary for the duration of the
syscall, so it's unlikely to stress vmalloc too much.
Note that the poll(2) syscall seems to use a linked list of order-0 pages, so
it doesn't need this kind of fallback.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
fs/select.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index 8ed9da50896a..8fe5bddbe99b 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -29,6 +29,7 @@
#include <linux/sched/rt.h>
#include <linux/freezer.h>
#include <net/busy_poll.h>
+#include <linux/vmalloc.h>
#include <asm/uaccess.h>
@@ -558,6 +559,7 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
struct fdtable *fdt;
/* Allocate small arguments on the stack to save memory and be faster */
long stack_fds[SELECT_STACK_ALLOC/sizeof(long)];
+ unsigned long alloc_size;
ret = -EINVAL;
if (n < 0)
@@ -580,10 +582,15 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
bits = stack_fds;
if (size > sizeof(stack_fds) / 6) {
/* Not enough space in on-stack array; must use kmalloc */
+ alloc_size = 6 * size;
ret = -ENOMEM;
- bits = kmalloc(6 * size, GFP_KERNEL);
- if (!bits)
- goto out_nofds;
+ bits = kmalloc(alloc_size, GFP_KERNEL|__GFP_NOWARN);
+ if (!bits && alloc_size > PAGE_SIZE) {
+ bits = vmalloc(alloc_size);
+
+ if (!bits)
+ goto out_nofds;
+ }
}
fds.in = bits;
fds.out = bits + size;
@@ -618,7 +625,7 @@ int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
out:
if (bits != stack_fds)
- kfree(bits);
+ kvfree(bits);
out_nofds:
return ret;
}
--
2.10.0
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH net-next v2 3/3] net: ethernet: mediatek: add the dts property to set if TRGMII supported on GMAC0
From: Sean Wang @ 2016-09-22 15:21 UTC (permalink / raw)
To: sergei.shtylyov
Cc: john, davem, nbd, netdev, linux-kernel, linux-mediatek, keyhaede
In-Reply-To: <b04e14ff-15db-9cc8-9490-204da8c5837d@cogentembedded.com>
Date: Thu, 22 Sep 2016 14:28:36 +0300, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
>Hello.
>
>On 9/22/2016 5:33 AM, sean.wang@mediatek.com wrote:
>
>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> Add the dts property for the capability if TRGMII supported on GAMC0
>>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>> Documentation/devicetree/bindings/net/mediatek-net.txt | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/mediatek-net.txt b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> index 6103e55..7111278 100644
>> --- a/Documentation/devicetree/bindings/net/mediatek-net.txt
>> +++ b/Documentation/devicetree/bindings/net/mediatek-net.txt
>> @@ -31,7 +31,10 @@ Optional properties:
>> Required properties:
>> - compatible: Should be "mediatek,eth-mac"
>> - reg: The number of the MAC
>> -- phy-handle: see ethernet.txt file in the same directory.
>> +- phy-handle: see ethernet.txt file in the same directory and
>> + the phy-mode "trgmii" required being provided when reg
>
> Since you've modified the generic parser of the "phy-mode" to add your
>"trgmii", you also need to update ethernet.txt...
>
>> + is equal to 0 and the MAC uses fixed-link to connect
>> + with inernal switch such as MT7530.
>
> Internal.
>
thank you for taking time and effort to review this
I will fix it up
^ permalink raw reply
* Re: [PATCH net-next 2/3] udp: implement memory accounting helpers
From: Edward Cree @ 2016-09-22 15:21 UTC (permalink / raw)
To: Paolo Abeni, Eric Dumazet
Cc: netdev, David S. Miller, James Morris, Trond Myklebust,
Alexander Duyck, Daniel Borkmann, Eric Dumazet, Tom Herbert,
Hannes Frederic Sowa, linux-nfs
In-Reply-To: <1474540415.4845.69.camel@redhat.com>
On 22/09/16 11:33, Paolo Abeni wrote:
> Hi Eric,
>
> On Wed, 2016-09-21 at 16:31 -0700, Eric Dumazet wrote:
>> Also does inet_diag properly give the forward_alloc to user ?
>>
>> $ ss -mua
>> State Recv-Q Send-Q Local Address:Port Peer Addres
>> s:Port
>> UNCONN 51584 0 *:52460 *:*
>> skmem:(r51584,rb327680,t0,tb327680,f1664,w0,o0,bl0,d575)
> Thank you very much for reviewing this!
>
> My bad, there is still a race which leads to temporary negative values
> of fwd. I feel the fix is trivial but it needs some investigation.
>
>> Couldn't we instead use an union of an atomic_t and int for
>> sk->sk_forward_alloc ?
> That was our first attempt, but we had some issue on mem scheduling; if
> we use:
>
> if (atomic_sub_return(size, &sk->sk_forward_alloc_atomic) < 0) {
> // fwd alloc
> }
>
> that leads to inescapable, temporary, negative value for
> sk->sk_forward_alloc.
>
> Another option would be:
>
> again:
> fwd = atomic_read(&sk->sk_forward_alloc_atomic);
> if (fwd > size) {
> if (atomic_cmpxchg(&sk->sk_forward_alloc_atomic, fwd, fwd - size) != fwd)
> goto again;
> } else
> // fwd alloc
>
> which would be bad under high contention.
Apologies if I'm misunderstanding the problem, but couldn't you have two
atomic_t fields, 'internal' and 'external' forward_alloc. Then
if (atomic_sub_return(size, &sk->sk_forward_alloc_internal) < 0) {
atomic_sub(size, &sk->sk_forward_alloc);
// fwd alloc
} else {
atomic_add(size, &sk->sk_forward_alloc_internal);
}
or something like that. Then sk->sk_forward_alloc never sees a negative
value, and is always >= sk->sk_forward_alloc_internal. Of course places
that go the other way would have to add to sk->sk_forward_alloc first,
before adding to sk->sk_forward_alloc_internal, to maintain that invariant.
Would that help matters at all?
-Ed
^ permalink raw reply
* Re: [PATCH] net: VRF: Fix receiving multicast traffic
From: David Ahern @ 2016-09-22 15:14 UTC (permalink / raw)
To: Mark Tomlinson, netdev
In-Reply-To: <20160922041308.30848-1-mark.tomlinson@alliedtelesis.co.nz>
On 9/21/16 10:13 PM, Mark Tomlinson wrote:
> The previous patch to ensure that the original iif was used when
> checking for forwarding also meant that this same interface was used to
> determine whether multicast packets should be received or not. This was
> incorrect, and would cause multicast packets to be dropped.
>
> The fix here is to use skb->dev when checking multicast addresses.
> skb->dev has been set to the l3mdev by this point, so the check will be
> against that, rather than the ingress interface.
l3mdev devices do not support IPv4 multicast so checking mcast against that device should not be working at all. For that reason I was fine with the change in the previous patch. ie., you want the real ingress device there not the vrf device.
What test are you running that says your previous patch broke something?
^ permalink raw reply
* Re: [PATCH v6 5/6] net: ipv4, ipv6: run cgroup eBPF egress programs
From: Daniel Borkmann @ 2016-09-22 15:12 UTC (permalink / raw)
To: Pablo Neira Ayuso, Thomas Graf
Cc: Daniel Mack, htejun, ast, davem, kafai, fw, harald, netdev,
sargun, cgroups
In-Reply-To: <20160922120554.GA1738@salvia>
On 09/22/2016 02:05 PM, Pablo Neira Ayuso wrote:
> On Thu, Sep 22, 2016 at 11:54:11AM +0200, Thomas Graf wrote:
>> On 09/22/16 at 11:21am, Pablo Neira Ayuso wrote:
>>> I have a hard time to buy this new specific hook, I think we should
>>> shift focus of this debate, this is my proposal to untangle this:
>>>
>>> You add a net/netfilter/nft_bpf.c expression that allows you to run
>>> bpf programs from nf_tables. This expression can either run bpf
>>> programs in a similar fashion to tc+bpf or run the bpf program that
>>> you have attached to the cgroup.
>>
>> So for every packet processed, you want to require the user to load
>> and run a (unJITed) nft program acting as a wrapper to run a JITed
>> BPF program? What it the benefit of this model compared to what Daniel
>> is proposing? The hooking point is the same. This only introduces
>> additional per packet overhead in the fast path. Am I missing
>> something?
>
> Have a look at net/ipv4/netfilter/nft_chain_route_ipv4.c for instance.
> In your case, you have to add a new chain type:
>
> static const struct nf_chain_type nft_chain_bpf = {
> .name = "bpf",
> .type = NFT_CHAIN_T_BPF,
> ...
> .hooks = {
> [NF_INET_LOCAL_IN] = nft_do_bpf,
> [NF_INET_LOCAL_OUT] = nft_do_bpf,
> [NF_INET_FORWARD] = nft_do_bpf,
> [NF_INET_PRE_ROUTING] = nft_do_bpf,
> [NF_INET_POST_ROUTING] = nft_do_bpf,
> },
> };
>
> nft_do_bpf() is the raw netfilter hook that you register, this hook
> will just execute to iterate over the list of bpf filters and run
> them.
>
> This new chain is created on demand, so no overhead if not needed, eg.
>
> nft add table bpf
> nft add chain bpf input { type bpf hook output priority 0\; }
>
> Then, you add a rule for each bpf program you want to run, just like
> tc+bpf.
But from a high-level point of view, this sounds like a huge hack to me,
in the sense that nft as a bytecode engine (from whole architecture I
mean) calls into another bytecode engine such as BPF as an extension. And
BPF code from there isn't using any of the features from nft besides being
invoked from the hook, yet it needs to be a hard dependency to be compiled
into the kernel when the only thing that is needed is to just load and
execute a BPF program that can do already everything needed by itself.
Also, if we look at history, at times such tings have been tried in the
past - just take tc calling into iptables as an example - I get goosebumps
(and probably you as well ;)), as the result looks worse than it would
have looked like when it would have been just kept separated.
I don't quite understand this detour, I mean, I would understand it when
the idea is that nft is using BPF as an intermediate to make use of all
the already existing JIT/offloading features that have been developed over
the years to not duplicate all the work for arch folks, but off-topic
in this discussion context here. I was hoping that nft would try to avoid
some of those exotic modules we have from xt, I would consider xt_bpf (no
offense ;)) as one of them; in the sense that in the context/model back
then of xt it made sense as most xt modules were kind of hard-coded, but
that was overcome with nft itself. So I'm not really keen on nft_bpf
idea besides that it also doesn't use anything else other than the hooks
themselves for what is proposed. Really, both are two different worlds
with different programming models and use-cases and it doesn't really make
sense to brute-force them together.
> Benefits are, rewording previous email:
>
> * You get access to all of the existing netfilter hooks in one go
> to run bpf programs. No need for specific redundant hooks. This
> provides raw access to the netfilter hook, you define the little
> code that your hook runs before you bpf run invocation. So there
> is *no need to bloat the stack with more hooks, we use what we
> have.*
But also this doesn't really address the fundamental underlying problem
that is discussed here. nft doesn't even have cgroups v2 support, only
xt_cgroups has it so far, but even if it would have it, then it's still
a scalability issue that this model has over what is being proposed by
Daniel, since you still need to test linearly wrt cgroups v2 membership,
whereas in the set that is proposed it's integral part of cgroups and can
be extended further, also for non-networking users to use this facility.
Or would the idea be that the current netfilter hooks would be redone in
a way that they are generic enough so that any other user could make use
of it independent of netfilter?
Thanks,
Daniel
^ permalink raw reply
* Re: [patch net-next 3/6] mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls
From: David Ahern @ 2016-09-22 15:12 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, idosch, eladr, yotamg, nogahf, ogerlitz, roopa,
nikolay, linville, andy, f.fainelli, jhs, vivien.didelot, andrew,
ivecera, kaber, john
In-Reply-To: <20160922150551.GE1830@nanopsycho.orion>
On 9/22/16 9:05 AM, Jiri Pirko wrote:
> Thu, Sep 22, 2016 at 04:58:20PM CEST, dsa@cumulusnetworks.com wrote:
>> On 9/21/16 5:53 AM, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@mellanox.com>
>>>
>>> Until now, in order to offload a FIB entry to HW we use switchdev op.
>>> However that has limits. Mainly in case we need to make the HW aware of
>>> all route prefixes configured in kernel. HW needs to know those in order
>>> to properly trap appropriate packets and pass the to kernel to do
>>> the forwarding. Abort mechanism is now handled within the mlxsw driver.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>>> ---
>>> drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 9 +-
>>> .../net/ethernet/mellanox/mlxsw/spectrum_router.c | 386 ++++++++++++---------
>>> .../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 -
>>> 3 files changed, 216 insertions(+), 188 deletions(-)
>>>
>>>
>>
>>
>> Make the move to fib notifiers and the removal of switchdev code separate patches. It will make the fib notifier change easier to follow.
>
> Since I'm changing one iface by another, I don't see how I can split it.
>
Make the previous API dead and remove it in a separate patch.
The init/fini code changes -- just a code move, no logical changes? then make it a separate patch - or don't move it at all and just add prototypes.
Some important code blocks are hard to read through the diffs as they are now.
^ permalink raw reply
* Re: [patch net-next 3/6] mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls
From: Jiri Pirko @ 2016-09-22 15:05 UTC (permalink / raw)
To: David Ahern
Cc: netdev, davem, idosch, eladr, yotamg, nogahf, ogerlitz, roopa,
nikolay, linville, andy, f.fainelli, jhs, vivien.didelot, andrew,
ivecera, kaber, john
In-Reply-To: <5206ebd6-89c1-ec44-8b98-c5109ebe6140@cumulusnetworks.com>
Thu, Sep 22, 2016 at 04:58:20PM CEST, dsa@cumulusnetworks.com wrote:
>On 9/21/16 5:53 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Until now, in order to offload a FIB entry to HW we use switchdev op.
>> However that has limits. Mainly in case we need to make the HW aware of
>> all route prefixes configured in kernel. HW needs to know those in order
>> to properly trap appropriate packets and pass the to kernel to do
>> the forwarding. Abort mechanism is now handled within the mlxsw driver.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 9 +-
>> .../net/ethernet/mellanox/mlxsw/spectrum_router.c | 386 ++++++++++++---------
>> .../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 -
>> 3 files changed, 216 insertions(+), 188 deletions(-)
>>
>>
>
>
>Make the move to fib notifiers and the removal of switchdev code separate patches. It will make the fib notifier change easier to follow.
Since I'm changing one iface by another, I don't see how I can split it.
^ permalink raw reply
* Re: [patch net-next 3/6] mlxsw: spectrum_router: Use FIB notifications instead of switchdev calls
From: David Ahern @ 2016-09-22 14:58 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, idosch, eladr, yotamg, nogahf, ogerlitz, roopa, nikolay,
linville, andy, f.fainelli, jhs, vivien.didelot, andrew, ivecera,
kaber, john
In-Reply-To: <1474458794-5512-4-git-send-email-jiri@resnulli.us>
On 9/21/16 5:53 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Until now, in order to offload a FIB entry to HW we use switchdev op.
> However that has limits. Mainly in case we need to make the HW aware of
> all route prefixes configured in kernel. HW needs to know those in order
> to properly trap appropriate packets and pass the to kernel to do
> the forwarding. Abort mechanism is now handled within the mlxsw driver.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 9 +-
> .../net/ethernet/mellanox/mlxsw/spectrum_router.c | 386 ++++++++++++---------
> .../ethernet/mellanox/mlxsw/spectrum_switchdev.c | 9 -
> 3 files changed, 216 insertions(+), 188 deletions(-)
>
>
Make the move to fib notifiers and the removal of switchdev code separate patches. It will make the fib notifier change easier to follow.
^ permalink raw reply
* Re: [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Eric Dumazet @ 2016-09-22 14:54 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: David S. Miller, Jamal Hadi Salim, WANG Cong, Eric Dumazet,
netdev, Shmulik Ladkani
In-Reply-To: <1474550512-7552-5-git-send-email-shmulik.ladkani@gmail.com>
On Thu, 2016-09-22 at 16:21 +0300, Shmulik Ladkani wrote:
> From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
>
> Up until now, 'action mirred' supported only egress actions (either
> TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).
>
> This patch implements the corresponding ingress actions
> TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
>
> This allows attaching filters whose target is to hand matching skbs into
> the rx processing of a specified device.
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> Was wondering, whether netif_receive_skb or dev_forward_skb should be
> used for the rx bouncing. Used netif_receive_skb as in ifb device.
Hmm... we probably need to apply the full rcu protection before this
patch.
https://patchwork.ozlabs.org/patch/667680/
^ permalink raw reply
* [PATCH net-next] sfc: check async completer is !NULL before calling
From: Bert Kenward @ 2016-09-22 14:47 UTC (permalink / raw)
To: Dave Miller; +Cc: netdev, Solarflare Linux Maintainers
Add a NULL check before calling asynchronous MCDI completion functions
during device removal.
Fixes: 7014d7f6 ("sfc: allow asynchronous MCDI without completion function")
Signed-off-by: Bert Kenward <bkenward@solarflare.com>
---
drivers/net/ethernet/sfc/mcdi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
index 9fbc12a..2415209 100644
--- a/drivers/net/ethernet/sfc/mcdi.c
+++ b/drivers/net/ethernet/sfc/mcdi.c
@@ -1156,7 +1156,8 @@ void efx_mcdi_flush_async(struct efx_nic *efx)
* acquired locks in the wrong order.
*/
list_for_each_entry_safe(async, next, &mcdi->async_list, list) {
- async->complete(efx, async->cookie, -ENETDOWN, NULL, 0);
+ if (async->complete)
+ async->complete(efx, async->cookie, -ENETDOWN, NULL, 0);
list_del(&async->list);
kfree(async);
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] net: rtnl_register in net_ns_init need rtnl_lock
From: Eric Dumazet @ 2016-09-22 14:48 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev
In-Reply-To: <4c1b9aba-e6e8-babc-1bbe-aef2bc0bca53@stressinduktion.org>
On Thu, 2016-09-22 at 15:41 +0200, Hannes Frederic Sowa wrote:
> I found this during working on the file and actually saw no live issues
> (belonged to another series which I just split up).
>
> I don't think it is a big issue but wanted the writes to the
> rtnl_msg_handlers array to be strictly serialized. I was working on
> adding this to other places, too. Maybe better for net-next even?
Sure, and you probably could enforce an ASSERT_RTNL() in rtnl_register()
to catch all offenders.
^ permalink raw reply
* Re: [PATCH RFC 1/3] xdp: Infrastructure to generalize XDP
From: Eric Dumazet @ 2016-09-22 14:46 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Tom Herbert, Thomas Graf, David S. Miller,
Linux Kernel Network Developers, Kernel Team, Tariq Toukan,
Brenden Blanco, Alexei Starovoitov
In-Reply-To: <20160922151403.24648381@redhat.com>
On Thu, 2016-09-22 at 15:14 +0200, Jesper Dangaard Brouer wrote:
> On Wed, 21 Sep 2016 21:56:58 +0200
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
> > > > I'm not opposed to running non-BPF code at XDP. I'm against adding
> > > > a linked list of hook consumers.
> >
> > I also worry about the performance impact of a linked list. We should
> > simple benchmark it instead of discussing it! ;-)
>
> (Note, there are some stability issue with this RFC patchset, when
> removing the xdp program, that I had to workaround/patch)
>
>
> I've started benchmarking this and I only see added cost of 2.89ns from
> these patches, at these crazy speeds it does correspond to -485Kpps.
I claim the methodology is too biased.
At full speed, all the extra code is hot in caches, and your core has
full access to memory bus anyway. Even branch predictor has fresh
information.
Now, in a mixed workload, where all cores compete to access L2/L3 and
RAM, things might be very different.
Testing icache/dcache pressure is not a matter of measuring how many
Kpps you add or remove on a hot path.
A latency test, when other cpus are busy reading/writing all over
memory, and your caches are cold, would be useful.
^ permalink raw reply
* Re: [PATCH net] net: rtnl_register in net_ns_init need rtnl_lock
From: Hannes Frederic Sowa @ 2016-09-22 13:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1474549384.23058.94.camel@edumazet-glaptop3.roam.corp.google.com>
On 22.09.2016 15:03, Eric Dumazet wrote:
> On Thu, 2016-09-22 at 13:03 +0200, Hannes Frederic Sowa wrote:
>> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> ---
>> net/core/net_namespace.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index 2c2eb1b629b11d..a2ace299f28355 100644
>> --- a/net/core/net_namespace.c
>> +++ b/net/core/net_namespace.c
>> @@ -758,9 +758,11 @@ static int __init net_ns_init(void)
>>
>> register_pernet_subsys(&net_ns_ops);
>>
>> + rtnl_lock();
>> rtnl_register(PF_UNSPEC, RTM_NEWNSID, rtnl_net_newid, NULL, NULL);
>> rtnl_register(PF_UNSPEC, RTM_GETNSID, rtnl_net_getid, rtnl_net_dumpid,
>> NULL);
>> + rtnl_unlock();
>>
>> return 0;
>> }
>
> Hi Hannes
>
> Why is this needed here, and not in other places ?
I found this during working on the file and actually saw no live issues
(belonged to another series which I just split up).
I don't think it is a big issue but wanted the writes to the
rtnl_msg_handlers array to be strictly serialized. I was working on
adding this to other places, too. Maybe better for net-next even?
Theoretically we would need to add a memory barriers to make sure we
don't publish uninitialized memory into the array if concurrent readers
of the array want to find their function pointers.
> Hint : A changelog always help reviewers and future bug hunting.
I will add that to v2.
Thanks,
Hannes
^ permalink raw reply
* Re: [RFC v2 06/12] qedr: Add support for QP verbs
From: Leon Romanovsky @ 2016-09-22 13:32 UTC (permalink / raw)
To: Amrani, Ram
Cc: davem@davemloft.net, dledford@redhat.com, Elior, Ariel,
Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <SN1PR07MB2207C1EC2C5C86C607D0255AF8F60@SN1PR07MB2207.namprd07.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 599 bytes --]
On Wed, Sep 21, 2016 at 02:15:39PM +0000, Amrani, Ram wrote:
> > > include/uapi/rdma/providers/qedr-abi.h | 35 +
> >
> > Please be aware that the last version for ABI header files doesn't have "provider"
> > name in directory path (include/uapi/rdma/qedr-abi.h)
>
> OK. Note that I was using https://www.spinics.net/lists/linux-rdma/msg40414.html that dates to 7 days ago and it contains the "providers" folder.
> Can you point to the most recent patch?
http://marc.info/?l=linux-rdma&m=147419583317034&w=2
I'll post new version soon. It will add mthca driver to the list of
moved drivers.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
To: David S. Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
Shmulik Ladkani
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Up until now, 'action mirred' supported only egress actions (either
TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).
This patch implements the corresponding ingress actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
---
Was wondering, whether netif_receive_skb or dev_forward_skb should be
used for the rx bouncing. Used netif_receive_skb as in ifb device.
net/sched/act_mirred.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 42 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 28629d3..942120e 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -33,6 +33,25 @@
static LIST_HEAD(mirred_list);
static DEFINE_SPINLOCK(mirred_list_lock);
+static bool tcf_mirred_is_act_redirect(int action)
+{
+ return action == TCA_EGRESS_REDIR || action == TCA_INGRESS_REDIR;
+}
+
+static u32 tcf_mirred_act_direction(int action)
+{
+ switch (action) {
+ case TCA_EGRESS_REDIR:
+ case TCA_EGRESS_MIRROR:
+ return AT_EGRESS;
+ case TCA_INGRESS_REDIR:
+ case TCA_INGRESS_MIRROR:
+ return AT_INGRESS;
+ default:
+ BUG();
+ }
+}
+
static void tcf_mirred_release(struct tc_action *a, int bind)
{
struct tcf_mirred *m = to_mirred(a);
@@ -96,6 +115,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
switch (parm->eaction) {
case TCA_EGRESS_MIRROR:
case TCA_EGRESS_REDIR:
+ case TCA_INGRESS_REDIR:
+ case TCA_INGRESS_MIRROR:
break;
default:
if (exists)
@@ -157,7 +178,8 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
struct tcf_mirred *m = to_mirred(a);
struct net_device *dev;
struct sk_buff *skb2;
- int retval, err;
+ int retval, err = 0;
+ int mac_len;
u32 at;
tcf_lastuse_update(&m->tcf_tm);
@@ -182,23 +204,37 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
if (!skb2)
goto out;
- if (!(at & AT_EGRESS)) {
- if (m->tcfm_mac_header_xmit)
+ /* If action's target direction differs than filter's direction,
+ * and devices expect a mac header on xmit, then mac push/pull is
+ * needed.
+ */
+ if (at != tcf_mirred_act_direction(m->tcfm_eaction) &&
+ m->tcfm_mac_header_xmit) {
+ if (at & AT_EGRESS) {
+ /* caught at egress, act ingress: pull mac */
+ mac_len = skb_network_header(skb) - skb_mac_header(skb);
+ skb_pull_rcsum(skb2, mac_len);
+ } else {
+ /* caught at ingress, act egress: push mac */
skb_push_rcsum(skb2, skb->mac_len);
+ }
}
/* mirror is always swallowed */
- if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+ if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
skb2->skb_iif = skb->dev->ifindex;
skb2->dev = dev;
- err = dev_queue_xmit(skb2);
+ if (tcf_mirred_act_direction(m->tcfm_eaction) & AT_EGRESS)
+ err = dev_queue_xmit(skb2);
+ else
+ netif_receive_skb(skb2);
if (err) {
out:
qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
- if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
+ if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
retval = TC_ACT_SHOT;
}
rcu_read_unlock();
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 3/4] net/sched: tc_mirred: Rename public predicates 'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
To: David S. Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
Shmulik Ladkani, Hariprasad S, Jeff Kirsher, Saeed Mahameed,
Jiri Pirko, Ido Schimmel, Jakub Kicinski
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
These accessors are used in various drivers that support tc offloading,
to detect properties of a given 'tc_action'.
'is_tcf_mirred_redirect' tests that the action is TCA_EGRESS_REDIR.
'is_tcf_mirred_mirror' tests that the action is TCA_EGRESS_MIRROR.
As a prep towards supporting INGRESS redir/mirror, rename these
predicates to reflect their true meaning:
s/is_tcf_mirred_redirect/is_tcf_mirred_egress_redirect/
s/is_tcf_mirred_mirror/is_tcf_mirred_egress_mirror/
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Hariprasad S <hariprasad@chelsio.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Saeed Mahameed <saeedm@mellanox.com>
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Ido Schimmel <idosch@mellanox.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +++-
drivers/net/ethernet/netronome/nfp/nfp_net_offload.c | 2 +-
include/net/tc_act/tc_mirred.h | 4 ++--
6 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
index 49d2deb..52af62e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c
@@ -113,7 +113,7 @@ static int fill_action_fields(struct adapter *adap,
}
/* Re-direct to specified port in hardware. */
- if (is_tcf_mirred_redirect(a)) {
+ if (is_tcf_mirred_egress_redirect(a)) {
struct net_device *n_dev;
unsigned int i, index;
bool found = false;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d76bc1a..ba03da2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8425,7 +8425,7 @@ static int parse_tc_actions(struct ixgbe_adapter *adapter,
}
/* Redirect to a VF or a offloaded macvlan */
- if (is_tcf_mirred_redirect(a)) {
+ if (is_tcf_mirred_egress_redirect(a)) {
int ifindex = tcf_mirred_ifindex(a);
err = handle_redirect_action(adapter, ifindex, queue,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 22cfc4a..7301dff 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -383,7 +383,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct tcf_exts *exts,
continue;
}
- if (is_tcf_mirred_redirect(a)) {
+ if (is_tcf_mirred_egress_redirect(a)) {
int ifindex = tcf_mirred_ifindex(a);
struct net_device *out_dev;
struct mlx5e_priv *out_priv;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 80f27b5..c5bac29 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1237,8 +1237,10 @@ static int mlxsw_sp_port_add_cls_matchall(struct mlxsw_sp_port *mlxsw_sp_port,
tcf_exts_to_list(cls->exts, &actions);
list_for_each_entry(a, &actions, list) {
- if (!is_tcf_mirred_mirror(a) || protocol != htons(ETH_P_ALL))
+ if (!is_tcf_mirred_egress_mirror(a) ||
+ protocol != htons(ETH_P_ALL)) {
return -ENOTSUPP;
+ }
err = mlxsw_sp_port_add_cls_matchall_mirror(mlxsw_sp_port, cls,
a, ingress);
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
index 43f42f8..2b1fa527 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_offload.c
@@ -128,7 +128,7 @@ nfp_net_bpf_get_act(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
if (is_tcf_gact_shot(a))
return NN_ACT_TC_DROP;
- if (is_tcf_mirred_redirect(a) &&
+ if (is_tcf_mirred_egress_redirect(a) &&
tcf_mirred_ifindex(a) == nn->netdev->ifindex)
return NN_ACT_TC_REDIR;
}
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 5275158..7b72c6b 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -14,7 +14,7 @@ struct tcf_mirred {
};
#define to_mirred(a) ((struct tcf_mirred *)a)
-static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_redirect(const struct tc_action *a)
{
#ifdef CONFIG_NET_CLS_ACT
if (a->ops && a->ops->type == TCA_ACT_MIRRED)
@@ -23,7 +23,7 @@ static inline bool is_tcf_mirred_redirect(const struct tc_action *a)
return false;
}
-static inline bool is_tcf_mirred_mirror(const struct tc_action *a)
+static inline bool is_tcf_mirred_egress_mirror(const struct tc_action *a)
{
#ifdef CONFIG_NET_CLS_ACT
if (a->ops && a->ops->type == TCA_ACT_MIRRED)
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 2/4] net/sched: act_mirred: Refactor detection whether dev needs xmit at mac header
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
To: David S. Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
Shmulik Ladkani
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Move detection logic that tests whether device expects skb data to point
to mac_header upon xmit into a function.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
net/sched/act_mirred.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 7b03b13..28629d3 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -54,6 +54,20 @@ static const struct nla_policy mirred_policy[TCA_MIRRED_MAX + 1] = {
static int mirred_net_id;
static struct tc_action_ops act_mirred_ops;
+static int dev_is_mac_header_xmit(const struct net_device *dev)
+{
+ switch (dev->type) {
+ case ARPHRD_TUNNEL:
+ case ARPHRD_TUNNEL6:
+ case ARPHRD_SIT:
+ case ARPHRD_IPGRE:
+ case ARPHRD_VOID:
+ case ARPHRD_NONE:
+ return 0;
+ }
+ return 1;
+}
+
static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
int bind)
@@ -95,19 +109,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
tcf_hash_release(*a, bind);
return -ENODEV;
}
- switch (dev->type) {
- case ARPHRD_TUNNEL:
- case ARPHRD_TUNNEL6:
- case ARPHRD_SIT:
- case ARPHRD_IPGRE:
- case ARPHRD_VOID:
- case ARPHRD_NONE:
- mac_header_xmit = 0;
- break;
- default:
- mac_header_xmit = 1;
- break;
- }
+ mac_header_xmit = dev_is_mac_header_xmit(dev);
} else {
dev = NULL;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
To: David S. Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
Shmulik Ladkani
In-Reply-To: <1474550512-7552-1-git-send-email-shmulik.ladkani@gmail.com>
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
'tcfm_ok_push' specifies whether a mac_len sized push is needed upon
egress to the target device (if action is performed at ingress).
Rename it to 'tcfm_mac_header_xmit' as this is actually an attribute of
the target device.
This allows to decouple the attribute from the action to be taken.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
include/net/tc_act/tc_mirred.h | 2 +-
net/sched/act_mirred.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 62770ad..5275158 100644
--- a/include/net/tc_act/tc_mirred.h
+++ b/include/net/tc_act/tc_mirred.h
@@ -8,7 +8,7 @@ struct tcf_mirred {
struct tc_action common;
int tcfm_eaction;
int tcfm_ifindex;
- int tcfm_ok_push;
+ int tcfm_mac_header_xmit;
struct net_device __rcu *tcfm_dev;
struct list_head tcfm_list;
};
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 667dc38..7b03b13 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -63,7 +63,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct tc_mirred *parm;
struct tcf_mirred *m;
struct net_device *dev;
- int ret, ok_push = 0;
+ int ret, mac_header_xmit = 0;
bool exists = false;
if (nla == NULL)
@@ -102,10 +102,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
case ARPHRD_IPGRE:
case ARPHRD_VOID:
case ARPHRD_NONE:
- ok_push = 0;
+ mac_header_xmit = 0;
break;
default:
- ok_push = 1;
+ mac_header_xmit = 1;
break;
}
} else {
@@ -136,7 +136,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
dev_put(rcu_dereference_protected(m->tcfm_dev, 1));
dev_hold(dev);
rcu_assign_pointer(m->tcfm_dev, dev);
- m->tcfm_ok_push = ok_push;
+ m->tcfm_mac_header_xmit = mac_header_xmit;
}
if (ret == ACT_P_CREATED) {
@@ -181,7 +181,7 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
goto out;
if (!(at & AT_EGRESS)) {
- if (m->tcfm_ok_push)
+ if (m->tcfm_mac_header_xmit)
skb_push_rcsum(skb2, skb->mac_len);
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 0/4] act_mirred: Ingress actions support
From: Shmulik Ladkani @ 2016-09-22 13:21 UTC (permalink / raw)
To: David S. Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, netdev,
Shmulik Ladkani
This patch series implements action mirred 'ingress' actions
TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
This allows attaching filters whose target is to hand matching skbs into
the rx processing of a specified device.
Shmulik Ladkani (4):
net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit
net/sched: act_mirred: Refactor detection whether dev needs xmit at
mac header
net/sched: tc_mirred: Rename public predicates
'is_tcf_mirred_redirect' and 'is_tcf_mirred_mirror'
net/sched: act_mirred: Implement ingress actions
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 +-
.../net/ethernet/netronome/nfp/nfp_net_offload.c | 2 +-
include/net/tc_act/tc_mirred.h | 6 +-
net/sched/act_mirred.c | 80 ++++++++++++++++------
7 files changed, 69 insertions(+), 29 deletions(-)
--
1.9.1
^ 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