* [PATCH net-next 3/4] net: dsa: b53: implement DSA port fast ageing
From: Vivien Didelot @ 2016-09-22 20:49 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, John Crispin, Vivien Didelot
In-Reply-To: <20160922204924.16229-1-vivien.didelot@savoirfairelinux.com>
Remove the fast ageing logic from b53_br_set_stp_state and implement the
new DSA switch port_fast_age operation instead.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
drivers/net/dsa/b53/b53_common.c | 31 +++++++++++--------------------
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 1a492c0..64be66d 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1402,16 +1402,12 @@ static void b53_br_leave(struct dsa_switch *ds, int port)
}
}
-static void b53_br_set_stp_state(struct dsa_switch *ds, int port,
- u8 state)
+static void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state)
{
struct b53_device *dev = ds->priv;
- u8 hw_state, cur_hw_state;
+ u8 hw_state;
u8 reg;
- b53_read8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), ®);
- cur_hw_state = reg & PORT_CTRL_STP_STATE_MASK;
-
switch (state) {
case BR_STATE_DISABLED:
hw_state = PORT_CTRL_DIS_STATE;
@@ -1433,26 +1429,20 @@ static void b53_br_set_stp_state(struct dsa_switch *ds, int port,
return;
}
- /* Fast-age ARL entries if we are moving a port from Learning or
- * Forwarding (cur_hw_state) state to Disabled, Blocking or Listening
- * state (hw_state)
- */
- if (cur_hw_state != hw_state) {
- if (cur_hw_state >= PORT_CTRL_LEARN_STATE &&
- hw_state <= PORT_CTRL_LISTEN_STATE) {
- if (b53_fast_age_port(dev, port)) {
- dev_err(ds->dev, "fast ageing failed\n");
- return;
- }
- }
- }
-
b53_read8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), ®);
reg &= ~PORT_CTRL_STP_STATE_MASK;
reg |= hw_state;
b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), reg);
}
+static void b53_br_fast_age(struct dsa_switch *ds, int port, u8 state)
+{
+ struct b53_device *dev = ds->priv;
+
+ if (b53_fast_age_port(dev, port))
+ dev_err(ds->dev, "fast ageing failed\n");
+}
+
static enum dsa_tag_protocol b53_get_tag_protocol(struct dsa_switch *ds)
{
return DSA_TAG_PROTO_NONE;
@@ -1472,6 +1462,7 @@ static struct dsa_switch_ops b53_switch_ops = {
.port_bridge_join = b53_br_join,
.port_bridge_leave = b53_br_leave,
.port_stp_state_set = b53_br_set_stp_state,
+ .port_fast_age = b53_br_fast_age,
.port_vlan_filtering = b53_vlan_filtering,
.port_vlan_prepare = b53_vlan_prepare,
.port_vlan_add = b53_vlan_add,
--
2.10.0
^ permalink raw reply related
* [PATCH net-next 2/4] net: dsa: add port fast ageing
From: Vivien Didelot @ 2016-09-22 20:49 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, John Crispin, Vivien Didelot
In-Reply-To: <20160922204924.16229-1-vivien.didelot@savoirfairelinux.com>
Today the DSA drivers are in charge of flushing the MAC addresses
associated to a port when its STP state changes from Learning or
Forwarding, to Disabled or Blocking or Listening.
This makes the drivers more complex and hides the generic switch logic.
Introduce a new optional port_fast_age operation to dsa_switch_ops, to
move this logic to the DSA layer and keep drivers simple.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
include/net/dsa.h | 2 ++
net/dsa/slave.c | 18 ++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 7556646..b122196 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -143,6 +143,7 @@ struct dsa_port {
struct net_device *netdev;
struct device_node *dn;
unsigned int ageing_time;
+ u8 stp_state;
};
struct dsa_switch {
@@ -339,6 +340,7 @@ struct dsa_switch_ops {
void (*port_bridge_leave)(struct dsa_switch *ds, int port);
void (*port_stp_state_set)(struct dsa_switch *ds, int port,
u8 state);
+ void (*port_fast_age)(struct dsa_switch *ds, int port);
/*
* VLAN support
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index fd78d4c..6b1282c 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -71,8 +71,26 @@ static inline bool dsa_port_is_bridged(struct dsa_slave_priv *p)
static void dsa_port_set_stp_state(struct dsa_switch *ds, int port, u8 state)
{
+ struct dsa_port *dp = &ds->ports[port];
+
if (ds->ops->port_stp_state_set)
ds->ops->port_stp_state_set(ds, port, state);
+
+ if (ds->ops->port_fast_age) {
+ /* Fast age FDB entries or flush appropriate forwarding database
+ * for the given port, if we are moving it from Learning or
+ * Forwarding state, to Disabled or Blocking or Listening state.
+ */
+
+ if ((dp->stp_state == BR_STATE_LEARNING ||
+ dp->stp_state == BR_STATE_FORWARDING) &&
+ (state == BR_STATE_DISABLED ||
+ state == BR_STATE_BLOCKING ||
+ state == BR_STATE_LISTENING))
+ ds->ops->port_fast_age(ds, port);
+ }
+
+ dp->stp_state = state;
}
static int dsa_slave_open(struct net_device *dev)
--
2.10.0
^ permalink raw reply related
* [PATCH net-next 1/4] net: dsa: add port STP state helper
From: Vivien Didelot @ 2016-09-22 20:49 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, John Crispin, Vivien Didelot
In-Reply-To: <20160922204924.16229-1-vivien.didelot@savoirfairelinux.com>
Add a void helper to set the STP state of a port, checking first if the
required routine is provided by the driver.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/slave.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 9ecbe78..fd78d4c 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -69,6 +69,12 @@ static inline bool dsa_port_is_bridged(struct dsa_slave_priv *p)
return !!p->bridge_dev;
}
+static void dsa_port_set_stp_state(struct dsa_switch *ds, int port, u8 state)
+{
+ if (ds->ops->port_stp_state_set)
+ ds->ops->port_stp_state_set(ds, port, state);
+}
+
static int dsa_slave_open(struct net_device *dev)
{
struct dsa_slave_priv *p = netdev_priv(dev);
@@ -104,8 +110,7 @@ static int dsa_slave_open(struct net_device *dev)
goto clear_promisc;
}
- if (ds->ops->port_stp_state_set)
- ds->ops->port_stp_state_set(ds, p->port, stp_state);
+ dsa_port_set_stp_state(ds, p->port, stp_state);
if (p->phy)
phy_start(p->phy);
@@ -147,8 +152,7 @@ static int dsa_slave_close(struct net_device *dev)
if (ds->ops->port_disable)
ds->ops->port_disable(ds, p->port, p->phy);
- if (ds->ops->port_stp_state_set)
- ds->ops->port_stp_state_set(ds, p->port, BR_STATE_DISABLED);
+ dsa_port_set_stp_state(ds, p->port, BR_STATE_DISABLED);
return 0;
}
@@ -354,7 +358,7 @@ static int dsa_slave_stp_state_set(struct net_device *dev,
if (switchdev_trans_ph_prepare(trans))
return ds->ops->port_stp_state_set ? 0 : -EOPNOTSUPP;
- ds->ops->port_stp_state_set(ds, p->port, attr->u.stp_state);
+ dsa_port_set_stp_state(ds, p->port, attr->u.stp_state);
return 0;
}
@@ -556,8 +560,7 @@ static void dsa_slave_bridge_port_leave(struct net_device *dev)
/* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
* so allow it to be in BR_STATE_FORWARDING to be kept functional
*/
- if (ds->ops->port_stp_state_set)
- ds->ops->port_stp_state_set(ds, p->port, BR_STATE_FORWARDING);
+ dsa_port_set_stp_state(ds, p->port, BR_STATE_FORWARDING);
}
static int dsa_slave_port_attr_get(struct net_device *dev,
--
2.10.0
^ permalink raw reply related
* Re: [PATCH net-next 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-09-22 20:37 UTC (permalink / raw)
To: Paolo Abeni
Cc: Edward Cree, netdev, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, linux-nfs
In-Reply-To: <1474576448.28155.7.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, 2016-09-22 at 13:34 -0700, Eric Dumazet wrote:
> On Thu, 2016-09-22 at 22:27 +0200, Paolo Abeni wrote:
> > On Thu, 2016-09-22 at 09:30 -0700, Eric Dumazet wrote:
> > > On Thu, 2016-09-22 at 18:14 +0200, Paolo Abeni wrote:
> > >
> > > > 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.
> > >
> > > But do we want any additional atomic to begin with ?
> > >
> > > Given typical number of UDP sockets on a host, we could reserve/forward
> > > alloc at socket creation time, and when SO_RCVBUF is changed.
> >
> > That would be very efficient and would probably work on most scenario,
> > but if/when the system will reach udp memory pressure things will be
> > very bad: forward allocation on open() will fail and nobody will be able
> > to create any new udp socket, right ?
> >
>
> No, we could allow one page per socket (udp_mem[0]) and applications
> would still work.
I meant udp_rmem_min, not udp_mem[0]
udp_rmem_min - INTEGER
Minimal size of receive buffer used by UDP sockets in moderation.
Each UDP socket is able to use the size for receiving data, even if
total pages of UDP sockets exceed udp_mem pressure. The unit is byte.
Default: 1 page
^ permalink raw reply
* Re: [PATCH net-next 2/3] udp: implement memory accounting helpers
From: Eric Dumazet @ 2016-09-22 20:34 UTC (permalink / raw)
To: Paolo Abeni
Cc: Edward Cree, netdev, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, linux-nfs
In-Reply-To: <1474576020.7120.1.camel@redhat.com>
On Thu, 2016-09-22 at 22:27 +0200, Paolo Abeni wrote:
> On Thu, 2016-09-22 at 09:30 -0700, Eric Dumazet wrote:
> > On Thu, 2016-09-22 at 18:14 +0200, Paolo Abeni wrote:
> >
> > > 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.
> >
> > But do we want any additional atomic to begin with ?
> >
> > Given typical number of UDP sockets on a host, we could reserve/forward
> > alloc at socket creation time, and when SO_RCVBUF is changed.
>
> That would be very efficient and would probably work on most scenario,
> but if/when the system will reach udp memory pressure things will be
> very bad: forward allocation on open() will fail and nobody will be able
> to create any new udp socket, right ?
>
No, we could allow one page per socket (udp_mem[0]) and applications
would still work.
TCP has the notion of memory pressure, and behaves roughly the same in
this case (one skb is allowed to be received)
The other (fat) sockets could notice udp_memory_pressure is set and
start reclaiming their forward allocations for other sockets.
We have a counter of UDP sockets, so probably doable to compute
udp_mem[2]/number
Anyway, just an idea.
> We are working on a v2 incorporating the feedback of your previous email
> - still keeping the new udp_sock fields.
> It looks quite simpler than v1, will work reasonably well in memory
> pressure scenario, and performance are measurably better than v1, most
> probably comparable with the above solution, since usually no additional
> atomic operations (beyond sk_rmem_alloc updating) are performed on
> enqueue/dequeue.
^ permalink raw reply
* Re: [PATCH net-next 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-22 20:27 UTC (permalink / raw)
To: Eric Dumazet
Cc: Edward Cree, netdev, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, linux-nfs
In-Reply-To: <1474561848.23058.133.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, 2016-09-22 at 09:30 -0700, Eric Dumazet wrote:
> On Thu, 2016-09-22 at 18:14 +0200, Paolo Abeni wrote:
>
> > 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.
>
> But do we want any additional atomic to begin with ?
>
> Given typical number of UDP sockets on a host, we could reserve/forward
> alloc at socket creation time, and when SO_RCVBUF is changed.
That would be very efficient and would probably work on most scenario,
but if/when the system will reach udp memory pressure things will be
very bad: forward allocation on open() will fail and nobody will be able
to create any new udp socket, right ?
We are working on a v2 incorporating the feedback of your previous email
- still keeping the new udp_sock fields.
It looks quite simpler than v1, will work reasonably well in memory
pressure scenario, and performance are measurably better than v1, most
probably comparable with the above solution, since usually no additional
atomic operations (beyond sk_rmem_alloc updating) are performed on
enqueue/dequeue.
Paolo
^ permalink raw reply
* [PATCH v2] bpf: Set register type according to is_valid_access()
From: Mickaël Salaün @ 2016-09-22 19:56 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
Daniel Borkmann, Kees Cook, Sargun Dhillon, Tejun Heo, netdev
This fix a pointer leak when an unprivileged eBPF program read a pointer
value from the context. Even if is_valid_access() returns a pointer
type, the eBPF verifier replace it with UNKNOWN_VALUE. The register
value containing an address is then allowed to leak. Moreover, this
prevented unprivileged eBPF programs to use functions with (legitimate)
pointer arguments.
This bug is not an issue for now because the only unprivileged eBPF
program allowed is of type BPF_PROG_TYPE_SOCKET_FILTER and all the types
from its context are UNKNOWN_VALUE. However, this fix is important for
future unprivileged eBPF program types which could use pointers in their
context.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Fixes: 969bf05eb3ce ("bpf: direct packet access")
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kees Cook <keescook@chromium.org>
Acked-by: Sargun Dhillon <sargun@sargun.me>
---
kernel/bpf/verifier.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index daea765d72e6..adbc7c161ba5 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -795,9 +795,8 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
err = check_ctx_access(env, off, size, t, ®_type);
if (!err && t == BPF_READ && value_regno >= 0) {
mark_reg_unknown_value(state->regs, value_regno);
- if (env->allow_ptr_leaks)
- /* note that reg.[id|off|range] == 0 */
- state->regs[value_regno].type = reg_type;
+ /* note that reg.[id|off|range] == 0 */
+ state->regs[value_regno].type = reg_type;
}
} else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v1] bpf: Set register type according to is_valid_access()
From: Mickaël Salaün @ 2016-09-22 19:53 UTC (permalink / raw)
To: Daniel Borkmann, linux-kernel
Cc: Alexei Starovoitov, Andy Lutomirski, Kees Cook, Sargun Dhillon,
Tejun Heo, netdev
In-Reply-To: <57E433F0.90407@iogearbox.net>
[-- Attachment #1.1: Type: text/plain, Size: 2680 bytes --]
On 22/09/2016 21:41, Daniel Borkmann wrote:
> On 09/22/2016 08:35 PM, Mickaël Salaün wrote:
>> This fix a pointer leak when an unprivileged eBPF program read a pointer
>> value from the context. Even if is_valid_access() returns a pointer
>> type, the eBPF verifier replace it with UNKNOWN_VALUE. The register
>> value containing an address is then allowed to leak. Moreover, this
>> prevented unprivileged eBPF programs to use functions with (legitimate)
>> pointer arguments.
>>
>> This bug is not an issue for now because the only unprivileged eBPF
>> program allowed is of type BPF_PROG_TYPE_SOCKET_FILTER and all the types
>> from its context are UNKNOWN_VALUE. However, this fix is important for
>> future unprivileged eBPF program types which could use pointers in their
>> context.
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Fixes: 969bf05eb3ce ("bpf: direct packet access")
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Kees Cook <keescook@chromium.org>
>> Acked-by: Sargun Dhillon <sargun@sargun.me>
>> ---
>> kernel/bpf/verifier.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index daea765d72e6..0698ccd67715 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -794,10 +794,8 @@ static int check_mem_access(struct verifier_env
>> *env, u32 regno, int off,
>> }
>> err = check_ctx_access(env, off, size, t, ®_type);
>> if (!err && t == BPF_READ && value_regno >= 0) {
>> - mark_reg_unknown_value(state->regs, value_regno);
>> - if (env->allow_ptr_leaks)
>> - /* note that reg.[id|off|range] == 0 */
>> - state->regs[value_regno].type = reg_type;
>> + /* note that reg.[id|off|range] == 0 */
>> + state->regs[value_regno].type = reg_type;
>
> True that it's not an issue currently, since reg_type is only set for
> PTR_TO_PACKET/PTR_TO_PACKET_END in xdp and tc programs that can only be
> loaded as privileged. So not an issue for BPF_PROG_TYPE_SOCKET_FILTER.
>
> One thing I don't quite follow is why you remove the
> mark_reg_unknown_value()
> as this also clears imm? I think this could result in an actual verifier
> bug when it would reuse previous tracked imm value of that dst register?
Good catch, I missed the imm initialization. I'm going to send a new patch.
>
>> }
>>
>> } else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH v1] bpf: Set register type according to is_valid_access()
From: Daniel Borkmann @ 2016-09-22 19:41 UTC (permalink / raw)
To: Mickaël Salaün, linux-kernel
Cc: Alexei Starovoitov, Andy Lutomirski, Kees Cook, Sargun Dhillon,
Tejun Heo, netdev
In-Reply-To: <20160922183512.13576-1-mic@digikod.net>
On 09/22/2016 08:35 PM, Mickaël Salaün wrote:
> This fix a pointer leak when an unprivileged eBPF program read a pointer
> value from the context. Even if is_valid_access() returns a pointer
> type, the eBPF verifier replace it with UNKNOWN_VALUE. The register
> value containing an address is then allowed to leak. Moreover, this
> prevented unprivileged eBPF programs to use functions with (legitimate)
> pointer arguments.
>
> This bug is not an issue for now because the only unprivileged eBPF
> program allowed is of type BPF_PROG_TYPE_SOCKET_FILTER and all the types
> from its context are UNKNOWN_VALUE. However, this fix is important for
> future unprivileged eBPF program types which could use pointers in their
> context.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Fixes: 969bf05eb3ce ("bpf: direct packet access")
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Kees Cook <keescook@chromium.org>
> Acked-by: Sargun Dhillon <sargun@sargun.me>
> ---
> kernel/bpf/verifier.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index daea765d72e6..0698ccd67715 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -794,10 +794,8 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
> }
> err = check_ctx_access(env, off, size, t, ®_type);
> if (!err && t == BPF_READ && value_regno >= 0) {
> - mark_reg_unknown_value(state->regs, value_regno);
> - if (env->allow_ptr_leaks)
> - /* note that reg.[id|off|range] == 0 */
> - state->regs[value_regno].type = reg_type;
> + /* note that reg.[id|off|range] == 0 */
> + state->regs[value_regno].type = reg_type;
True that it's not an issue currently, since reg_type is only set for
PTR_TO_PACKET/PTR_TO_PACKET_END in xdp and tc programs that can only be
loaded as privileged. So not an issue for BPF_PROG_TYPE_SOCKET_FILTER.
One thing I don't quite follow is why you remove the mark_reg_unknown_value()
as this also clears imm? I think this could result in an actual verifier
bug when it would reuse previous tracked imm value of that dst register?
> }
>
> } else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
>
^ permalink raw reply
* [RFC] net: store port/representative id in metadata_dst
From: Jakub Kicinski @ 2016-09-22 19:26 UTC (permalink / raw)
To: netdev, Thomas Graf
Cc: Roopa Prabhu, Jiri Pirko, ogerlitz, John Fastabend,
sridhar.samudrala, ast, daniel, simon.horman, Paolo Abeni,
Pravin B Shelar, Jiri Benc, hannes, kubakici, Jakub Kicinski
Switches and modern SR-IOV enabled NICs may multiplex traffic from
representators and control messages over single set of hardware queues.
Control messages and muxed traffic may need ordered delivery.
Those requirements make it hard to comfortably use TC infrastructure
today unless we have a way of attaching metadata to skbs at the upper
device. Because single set of queues is used for many netdevs stopping
TC/sched queues of all of them reliably is impossible and lower
device has to retreat to returning NETDEV_TX_BUSY and usually
has to take extra locks on the fastpath.
This patch attempts to enable port/representative devs to attach metadata
to skbs which carry port id. This way representatives can be queueless
and all queuing can be performed at the lower netdev in the usual way.
Traffic arriving on the port/representative interfaces will be have
metadata attached and will subsequently be queued to the lower device
for transmission. The lower device should recognize the metadata and
translate it to HW specific format which is most likely either a special
header inserted before the network headers or descriptor/metadata fields.
Metadata is associated with the lower device by storing the netdev pointer
along with port id so that if TC decides to redirect or mirror the new
netdev will not try to interpret it.
This is mostly for SR-IOV devices since switches don't have lower
netdevs today.
Since I don't have any real user in the tree at this point please
allow me to present a trivial example use here:
void upper_init(struct upper *upper, struct lower *lower, unsigned int id)
{
upper->lower_dev = lower;
upper->dst_meta = metadata_dst_alloc(0, METADATA_HW_PORT_MUX,
GFP_KERNEL);
upper->dst_meta.u.lower_dev = lower->netdev;
upper->dst_meta.u.port_info.port_id = id;
}
int upper_tx(struct sk_buff *skb, struct net_device *netdev)
{
struct upper *upper = netdev_priv(netdev);
skb_dst_drop(skb);
skb_dst_set_noref(skb, upper->dst_meta);
return dev_queue_xmit(upper->lower_dev, skb);
}
int lower_tx(struct sk_buff *skb, struct net_device *netdev)
{
struct metadata_dst *md = skb_metadata_dst(skb);
struct lower *lower = netdev_priv(netdev);
if (md->type == METADATA_HW_PORT_MUX &&
md->u.lower_dev == netdev) {
/* use md->u.port_id to set port in
* descriptor/metadata/do encap
*/
}
...
}
Other approaches considered but found inferior:
- in-data tags - inserting tags into data will be confusing
to classifiers which start parsing from mac headers, also
in-band data is less perfect and allows sufficiently privileged
user to inject control messages from userspace (this is DSA model
- note that in SR-IOV switchdev mode I control both upper and lower
device which differs from DSA where lower device can be any MAC);
- per-VFR HW queues - requiring a queue per VF is a little wasteful and
less scalable, muxing allows us to use all PF queues to transmit
and receive with full RSS (this is model of existing SR-IOV switchdev
mode implementations);
- per-VFR TC queue - we could use per-VFR queue in the lower device,
tag traffic and TX on smaller set of HW queues but again scaling
would suffer, we would need to lock an extra queue and we have no
way to stop all queues when HW queues fill up reliably (this model
would piggy back on dev_queue_xmit_accel() to select queue).
Any comments, reactions would be much appreciated!
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/net/dst_metadata.h | 35 ++++++++++++++++++++++++++++-------
net/core/dst.c | 14 +++++++++-----
net/core/filter.c | 1 +
net/ipv4/ip_tunnel_core.c | 5 +++--
net/openvswitch/flow_netlink.c | 3 ++-
5 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
index 6965c8f68ade..6d7e1e4f3acd 100644
--- a/include/net/dst_metadata.h
+++ b/include/net/dst_metadata.h
@@ -5,10 +5,22 @@
#include <net/ip_tunnels.h>
#include <net/dst.h>
+enum metadata_type {
+ METADATA_IP_TUNNEL,
+ METADATA_HW_PORT_MUX,
+};
+
+struct hw_port_info {
+ struct netdevice *lower_dev;
+ u32 port_id;
+};
+
struct metadata_dst {
struct dst_entry dst;
+ enum metadata_type type;
union {
struct ip_tunnel_info tun_info;
+ struct hw_port_info port_info;
} u;
};
@@ -27,7 +39,7 @@ static inline struct ip_tunnel_info *skb_tunnel_info(struct sk_buff *skb)
struct metadata_dst *md_dst = skb_metadata_dst(skb);
struct dst_entry *dst;
- if (md_dst)
+ if (md_dst && md_dst->type == METADATA_IP_TUNNEL)
return &md_dst->u.tun_info;
dst = skb_dst(skb);
@@ -55,7 +67,14 @@ static inline int skb_metadata_dst_cmp(const struct sk_buff *skb_a,
a = (const struct metadata_dst *) skb_dst(skb_a);
b = (const struct metadata_dst *) skb_dst(skb_b);
- if (!a != !b || a->u.tun_info.options_len != b->u.tun_info.options_len)
+ if (!a != !b || a->type != b->type)
+ return 1;
+
+ if (a->type == METADATA_HW_PORT_MUX)
+ return memcmp(&a->u.port_info, &b->u.port_info,
+ sizeof(a->u.port_info));
+
+ if (a->u.tun_info.options_len != b->u.tun_info.options_len)
return 1;
return memcmp(&a->u.tun_info, &b->u.tun_info,
@@ -63,14 +82,16 @@ static inline int skb_metadata_dst_cmp(const struct sk_buff *skb_a,
}
void metadata_dst_free(struct metadata_dst *);
-struct metadata_dst *metadata_dst_alloc(u8 optslen, gfp_t flags);
-struct metadata_dst __percpu *metadata_dst_alloc_percpu(u8 optslen, gfp_t flags);
+struct metadata_dst *metadata_dst_alloc(u8 optslen, enum metadata_type type,
+ gfp_t flags);
+struct metadata_dst __percpu *
+metadata_dst_alloc_percpu(u8 optslen, enum metadata_type type, gfp_t flags);
static inline struct metadata_dst *tun_rx_dst(int md_size)
{
struct metadata_dst *tun_dst;
- tun_dst = metadata_dst_alloc(md_size, GFP_ATOMIC);
+ tun_dst = metadata_dst_alloc(md_size, METADATA_IP_TUNNEL, GFP_ATOMIC);
if (!tun_dst)
return NULL;
@@ -85,11 +106,11 @@ static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb)
int md_size;
struct metadata_dst *new_md;
- if (!md_dst)
+ if (!md_dst || md_dst->type != METADATA_IP_TUNNEL)
return ERR_PTR(-EINVAL);
md_size = md_dst->u.tun_info.options_len;
- new_md = metadata_dst_alloc(md_size, GFP_ATOMIC);
+ new_md = metadata_dst_alloc(md_size, METADATA_IP_TUNNEL, GFP_ATOMIC);
if (!new_md)
return ERR_PTR(-ENOMEM);
diff --git a/net/core/dst.c b/net/core/dst.c
index b5cbbe07f786..dc8c0c0b197b 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -367,7 +367,8 @@ static int dst_md_discard(struct sk_buff *skb)
return 0;
}
-static void __metadata_dst_init(struct metadata_dst *md_dst, u8 optslen)
+static void __metadata_dst_init(struct metadata_dst *md_dst,
+ enum metadata_type type, u8 optslen)
{
struct dst_entry *dst;
@@ -379,9 +380,11 @@ static void __metadata_dst_init(struct metadata_dst *md_dst, u8 optslen)
dst->output = dst_md_discard_out;
memset(dst + 1, 0, sizeof(*md_dst) + optslen - sizeof(*dst));
+ md_dst->type = type;
}
-struct metadata_dst *metadata_dst_alloc(u8 optslen, gfp_t flags)
+struct metadata_dst *metadata_dst_alloc(u8 optslen, enum metadata_type type,
+ gfp_t flags)
{
struct metadata_dst *md_dst;
@@ -389,7 +392,7 @@ struct metadata_dst *metadata_dst_alloc(u8 optslen, gfp_t flags)
if (!md_dst)
return NULL;
- __metadata_dst_init(md_dst, optslen);
+ __metadata_dst_init(md_dst, type, optslen);
return md_dst;
}
@@ -403,7 +406,8 @@ void metadata_dst_free(struct metadata_dst *md_dst)
kfree(md_dst);
}
-struct metadata_dst __percpu *metadata_dst_alloc_percpu(u8 optslen, gfp_t flags)
+struct metadata_dst __percpu *
+metadata_dst_alloc_percpu(u8 optslen, enum metadata_type type, gfp_t flags)
{
int cpu;
struct metadata_dst __percpu *md_dst;
@@ -414,7 +418,7 @@ struct metadata_dst __percpu *metadata_dst_alloc_percpu(u8 optslen, gfp_t flags)
return NULL;
for_each_possible_cpu(cpu)
- __metadata_dst_init(per_cpu_ptr(md_dst, cpu), optslen);
+ __metadata_dst_init(per_cpu_ptr(md_dst, cpu), type, optslen);
return md_dst;
}
diff --git a/net/core/filter.c b/net/core/filter.c
index 0920c2ac1d00..61536a7e932e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2386,6 +2386,7 @@ bpf_get_skb_set_tunnel_proto(enum bpf_func_id which)
* that is holding verifier mutex.
*/
md_dst = metadata_dst_alloc_percpu(IP_TUNNEL_OPTS_MAX,
+ METADATA_IP_TUNNEL,
GFP_KERNEL);
if (!md_dst)
return NULL;
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 777bc1883870..12ffbc4a4daa 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -145,10 +145,11 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md,
struct metadata_dst *res;
struct ip_tunnel_info *dst, *src;
- if (!md || md->u.tun_info.mode & IP_TUNNEL_INFO_TX)
+ if (!md || md->type != METADATA_IP_TUNNEL ||
+ md->u.tun_info.mode & IP_TUNNEL_INFO_TX)
return NULL;
- res = metadata_dst_alloc(0, flags);
+ res = metadata_dst_alloc(0, METADATA_IP_TUNNEL, flags);
if (!res)
return NULL;
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index ae25ded82b3b..c9971701d0af 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2072,7 +2072,8 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
if (start < 0)
return start;
- tun_dst = metadata_dst_alloc(key.tun_opts_len, GFP_KERNEL);
+ tun_dst = metadata_dst_alloc(key.tun_opts_len, METADATA_IP_TUNNEL,
+ GFP_KERNEL);
if (!tun_dst)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* CONGRATULATIONS !!!
From: Cheveron Texaco @ 2016-09-22 18:59 UTC (permalink / raw)
Attention Winner,
Congratulations !!! Your email Address has won you a substantial amount of 1 Million United States dollars in the just concluded Chevron Texaco Promo-Lottery.
You are to contact your claim agent for claims process at Email: chevron.claimsagent1401@gmail.com
Best Regards
Ms Stella Newton
^ permalink raw reply
* Re: [PATCH net] net: rtnl_register in net_ns_init need rtnl_lock
From: Hannes Frederic Sowa @ 2016-09-22 19:02 UTC (permalink / raw)
To: Cong Wang; +Cc: Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <CAM_iQpUsYLWxWvcuVzHt=Q9hRjZ1vKJHWjS6ZKWc0rjdtdOo8w@mail.gmail.com>
On Thu, Sep 22, 2016, at 19:20, Cong Wang wrote:
> > 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?
>
> But they are called during boot, why is it possible to have a parallel
> reader/writer at that time?
It also happens during module load time, which isn't synchronized with
rtnl_lock.
Bye,
Hannes
^ permalink raw reply
* [PATCH v3] tcp: fix wrong checksum calculation on MTU probing
From: Douglas Caetano dos Santos @ 2016-09-22 18:52 UTC (permalink / raw)
To: Sergei Shtylyov, David Miller; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <5ffc1020-a36f-75b4-ff25-4f58c243f803@cogentembedded.com>
With TCP MTU probing enabled and offload TX checksumming disabled,
tcp_mtu_probe() calculated the wrong checksum when a fragment being copied
into the probe's SKB had an odd length. This was caused by the direct use
of skb_copy_and_csum_bits() to calculate the checksum, as it pads the
fragment being copied, if needed. When this fragment was not the last, a
subsequent call used the previous checksum without considering this
padding.
The effect was a stale connection in one way, as even retransmissions
wouldn't solve the problem, because the checksum was never recalculated for
the full SKB length.
Signed-off-by: Douglas Caetano dos Santos <douglascs@taghos.com.br>
---
net/ipv4/tcp_output.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f53d0cc..2d32952 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1966,12 +1966,14 @@ static int tcp_mtu_probe(struct sock *sk)
len = 0;
tcp_for_write_queue_from_safe(skb, next, sk) {
copy = min_t(int, skb->len, probe_size - len);
- if (nskb->ip_summed)
+ if (nskb->ip_summed) {
skb_copy_bits(skb, 0, skb_put(nskb, copy), copy);
- else
- nskb->csum = skb_copy_and_csum_bits(skb, 0,
- skb_put(nskb, copy),
- copy, nskb->csum);
+ } else {
+ __wsum csum = skb_copy_and_csum_bits(skb, 0,
+ skb_put(nskb, copy),
+ copy, 0);
+ nskb->csum = csum_block_add(nskb->csum, csum, len);
+ }
if (skb->len <= copy) {
/* We've eaten all the data from this skb.
--
2.5.0
^ permalink raw reply related
* Re: [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Eric Dumazet @ 2016-09-22 18:42 UTC (permalink / raw)
To: Shmulik Ladkani
Cc: David S. Miller, Jamal Hadi Salim, WANG Cong, Eric Dumazet,
netdev
In-Reply-To: <20160922212745.789734c5@halley>
On Thu, 2016-09-22 at 21:27 +0300, Shmulik Ladkani wrote:
> On Thu, 22 Sep 2016 07:54:13 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Hmm... we probably need to apply the full rcu protection before this
> > patch.
> >
> > https://patchwork.ozlabs.org/patch/667680/
>
> Are you referring to order of application into net-next?
>
> This patch seems to present no new tcf_mirred_params members nor
> need-to-be-protected code regions (please, correct me if wrong).
> So it does not _depend_ on the 'full rcu fixes', does it?
No, simply a reminder that we run lockless there, so you might need to
read some control variables once, and in a consistent way.
(Or a concurrent writer could change params in the middle of the
function)
^ permalink raw reply
* Re: [PATCH v3 2/2] netfilter: Create revision 2 of xt_hashlimit to support higher pps rates
From: Vishwanath Pai @ 2016-09-22 18:39 UTC (permalink / raw)
To: Jan Engelhardt, pablo
Cc: kaber, kadlec, johunt, netfilter-devel, coreteam, netdev,
pai.vishwain
In-Reply-To: <alpine.LSU.2.20.1609221850530.27197@nerf40.vanv.qr>
Thanks for pointing this out, I will reorder the fields to:
struct hashlimit_cfg2 {
__u64 avg; /* Average secs between packets * scale */
__u64 burst;
__u32 mode; /* bitmask of XT_HASHLIMIT_HASH_* */
This should fix the hole and avoid padding.
-Vishwanath
On 09/22/2016 12:53 PM, Jan Engelhardt wrote:
> On Thursday 2016-09-22 18:43, Vishwanath Pai wrote:
>> >+struct hashlimit_cfg2 {
>> >+ __u32 mode; /* bitmask of XT_HASHLIMIT_HASH_* */
>> >+ __u64 avg; /* Average secs between packets * scale */
>> >+ __u64 burst; /* Period multiplier for upper limit. */
> This would have different sizes between i386 and x86_64,
> necessiting additional compat functions. It should be padded
> or reordered instead.
>
^ permalink raw reply
* [PATCH v1] bpf: Set register type according to is_valid_access()
From: Mickaël Salaün @ 2016-09-22 18:35 UTC (permalink / raw)
To: linux-kernel
Cc: Mickaël Salaün, Alexei Starovoitov, Andy Lutomirski,
Daniel Borkmann, Kees Cook, Sargun Dhillon, Tejun Heo, netdev
This fix a pointer leak when an unprivileged eBPF program read a pointer
value from the context. Even if is_valid_access() returns a pointer
type, the eBPF verifier replace it with UNKNOWN_VALUE. The register
value containing an address is then allowed to leak. Moreover, this
prevented unprivileged eBPF programs to use functions with (legitimate)
pointer arguments.
This bug is not an issue for now because the only unprivileged eBPF
program allowed is of type BPF_PROG_TYPE_SOCKET_FILTER and all the types
from its context are UNKNOWN_VALUE. However, this fix is important for
future unprivileged eBPF program types which could use pointers in their
context.
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Fixes: 969bf05eb3ce ("bpf: direct packet access")
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Kees Cook <keescook@chromium.org>
Acked-by: Sargun Dhillon <sargun@sargun.me>
---
kernel/bpf/verifier.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index daea765d72e6..0698ccd67715 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -794,10 +794,8 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
}
err = check_ctx_access(env, off, size, t, ®_type);
if (!err && t == BPF_READ && value_regno >= 0) {
- mark_reg_unknown_value(state->regs, value_regno);
- if (env->allow_ptr_leaks)
- /* note that reg.[id|off|range] == 0 */
- state->regs[value_regno].type = reg_type;
+ /* note that reg.[id|off|range] == 0 */
+ state->regs[value_regno].type = reg_type;
}
} else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Shmulik Ladkani @ 2016-09-22 18:27 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Jamal Hadi Salim, WANG Cong, Eric Dumazet,
netdev, Shmulik Ladkani
In-Reply-To: <1474556053.23058.111.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, 22 Sep 2016 07:54:13 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Hmm... we probably need to apply the full rcu protection before this
> patch.
>
> https://patchwork.ozlabs.org/patch/667680/
Are you referring to order of application into net-next?
This patch seems to present no new tcf_mirred_params members nor
need-to-be-protected code regions (please, correct me if wrong).
So it does not _depend_ on the 'full rcu fixes', does it?
Thanks,
Shmulik
^ permalink raw reply
* Re: XDP (eXpress Data Path) documentation
From: Jesper Dangaard Brouer via iovisor-dev @ 2016-09-22 18:16 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iovisor-dev-9jONkmmOlFHEE9lA1F8Ukti2O/JbrIOy@public.gmane.org
Cc: Nathan Willis, Alexei Starovoitov, Tom Herbert, Jonathan Corbet,
linux-doc-u79uwXL29TY76Z2rM5mHXA, Saeed Mahameed, Edward Cree
In-Reply-To: <20160920110844.661965be-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, 20 Sep 2016 11:08:44 +0200 Jesper Dangaard Brouer <brouer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> As promised, I've started documenting the XDP eXpress Data Path):
>
> [1] https://prototype-kernel.readthedocs.io/en/latest/networking/XDP/index.html
>
> IMHO the documentation have reached a stage where it is useful for the
> XDP project, BUT I request collaboration on improving the documentation
> from all. (Native English speakers are encouraged to send grammar fixes ;-))
I want to publicly thanks Edward Cree for being the first contributor
to the XDP documentation with formulation and grammar fixes.
Pulled and pushed:
https://github.com/netoptimizer/prototype-kernel/commit/fb6a3de95
Thanks!
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH v2 iproute2 net-next] tc: m_vlan: Add vlan modify action
From: Shmulik Ladkani @ 2016-09-22 18:01 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Jamal Hadi Salim, Jiri Pirko, Shmulik Ladkani
The 'vlan modify' action allows to replace an existing 802.1q tag
according to user provided settings.
It accepts same arguments as the 'vlan push' action.
For example, this replaces vid 6 with vid 5:
# tc filter add dev veth0 parent ffff: pref 1 protocol 802.1q \
basic match 'meta(vlan mask 0xfff eq 6)' \
action vlan modify id 5 continue
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
v2: Coding. No need to encapsule action_names[] access into a function
include/linux/tc_act/tc_vlan.h | 1 +
man/man8/tc-vlan.8 | 25 +++++++++++++++++++------
tc/m_vlan.c | 40 +++++++++++++++++++++++++++++++---------
3 files changed, 51 insertions(+), 15 deletions(-)
diff --git a/include/linux/tc_act/tc_vlan.h b/include/linux/tc_act/tc_vlan.h
index be72b6e384..bddb272b84 100644
--- a/include/linux/tc_act/tc_vlan.h
+++ b/include/linux/tc_act/tc_vlan.h
@@ -16,6 +16,7 @@
#define TCA_VLAN_ACT_POP 1
#define TCA_VLAN_ACT_PUSH 2
+#define TCA_VLAN_ACT_MODIFY 3
struct tc_vlan {
tc_gen;
diff --git a/man/man8/tc-vlan.8 b/man/man8/tc-vlan.8
index 4d0c5c8a15..af3de1c54e 100644
--- a/man/man8/tc-vlan.8
+++ b/man/man8/tc-vlan.8
@@ -6,7 +6,7 @@ vlan - vlan manipulation module
.in +8
.ti -8
.BR tc " ... " "action vlan" " { " pop " |"
-.IR PUSH " } [ " CONTROL " ]"
+.IR PUSH " | " MODIFY " } [ " CONTROL " ]"
.ti -8
.IR PUSH " := "
@@ -17,22 +17,30 @@ vlan - vlan manipulation module
.BI id " VLANID"
.ti -8
+.IR MODIFY " := "
+.BR modify " [ " protocol
+.IR VLANPROTO " ]"
+.BR " [ " priority
+.IR VLANPRIO " ] "
+.BI id " VLANID"
+
+.ti -8
.IR CONTROL " := { "
.BR reclassify " | " pipe " | " drop " | " continue " | " pass " }"
.SH DESCRIPTION
The
.B vlan
action allows to perform 802.1Q en- or decapsulation on a packet, reflected by
-the two operation modes
-.IR POP " and " PUSH .
+the operation modes
+.IR POP ", " PUSH " and " MODIFY .
The
.I POP
mode is simple, as no further information is required to just drop the
outer-most VLAN encapsulation. The
-.I PUSH
-mode on the other hand requires at least a
+.IR PUSH " and " MODIFY
+modes require at least a
.I VLANID
-and allows to optionally choose the
+and allow to optionally choose the
.I VLANPROTO
to use.
.SH OPTIONS
@@ -45,6 +53,11 @@ Encapsulation mode. Requires at least
.B id
option.
.TP
+.B modify
+Replace mode. Existing 802.1Q tag is replaced. Requires at least
+.B id
+option.
+.TP
.BI id " VLANID"
Specify the VLAN ID to encapsulate into.
.I VLANID
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index 05a63b48f1..b32f746015 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -19,10 +19,17 @@
#include "tc_util.h"
#include <linux/tc_act/tc_vlan.h>
+static const char * const action_names[] = {
+ [TCA_VLAN_ACT_POP] = "pop",
+ [TCA_VLAN_ACT_PUSH] = "push",
+ [TCA_VLAN_ACT_MODIFY] = "modify",
+};
+
static void explain(void)
{
fprintf(stderr, "Usage: vlan pop\n");
fprintf(stderr, " vlan push [ protocol VLANPROTO ] id VLANID [ priority VLANPRIO ] [CONTROL]\n");
+ fprintf(stderr, " vlan modify [ protocol VLANPROTO ] id VLANID [ priority VLANPRIO ] [CONTROL]\n");
fprintf(stderr, " VLANPROTO is one of 802.1Q or 802.1AD\n");
fprintf(stderr, " with default: 802.1Q\n");
fprintf(stderr, " CONTROL := reclassify | pipe | drop | continue | pass\n");
@@ -34,6 +41,11 @@ static void usage(void)
exit(-1);
}
+static bool has_push_attribs(int action)
+{
+ return action == TCA_VLAN_ACT_PUSH || action == TCA_VLAN_ACT_MODIFY;
+}
+
static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
int tca_id, struct nlmsghdr *n)
{
@@ -71,9 +83,17 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
return -1;
}
action = TCA_VLAN_ACT_PUSH;
+ } else if (matches(*argv, "modify") == 0) {
+ if (action) {
+ fprintf(stderr, "unexpected \"%s\" - action already specified\n",
+ *argv);
+ explain();
+ return -1;
+ }
+ action = TCA_VLAN_ACT_MODIFY;
} else if (matches(*argv, "id") == 0) {
- if (action != TCA_VLAN_ACT_PUSH) {
- fprintf(stderr, "\"%s\" is only valid for push\n",
+ if (!has_push_attribs(action)) {
+ fprintf(stderr, "\"%s\" is only valid for push/modify\n",
*argv);
explain();
return -1;
@@ -83,8 +103,8 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
invarg("id is invalid", *argv);
id_set = 1;
} else if (matches(*argv, "protocol") == 0) {
- if (action != TCA_VLAN_ACT_PUSH) {
- fprintf(stderr, "\"%s\" is only valid for push\n",
+ if (!has_push_attribs(action)) {
+ fprintf(stderr, "\"%s\" is only valid for push/modify\n",
*argv);
explain();
return -1;
@@ -94,8 +114,8 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
invarg("protocol is invalid", *argv);
proto_set = 1;
} else if (matches(*argv, "priority") == 0) {
- if (action != TCA_VLAN_ACT_PUSH) {
- fprintf(stderr, "\"%s\" is only valid for push\n",
+ if (!has_push_attribs(action)) {
+ fprintf(stderr, "\"%s\" is only valid for push/modify\n",
*argv);
explain();
return -1;
@@ -129,8 +149,9 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
}
}
- if (action == TCA_VLAN_ACT_PUSH && !id_set) {
- fprintf(stderr, "id needs to be set for push\n");
+ if (has_push_attribs(action) && !id_set) {
+ fprintf(stderr, "id needs to be set for %s\n",
+ action_names[action]);
explain();
return -1;
}
@@ -186,7 +207,8 @@ static int print_vlan(struct action_util *au, FILE *f, struct rtattr *arg)
fprintf(f, " pop");
break;
case TCA_VLAN_ACT_PUSH:
- fprintf(f, " push");
+ case TCA_VLAN_ACT_MODIFY:
+ fprintf(f, " %s", action_names[parm->v_action]);
if (tb[TCA_VLAN_PUSH_VLAN_ID]) {
val = rta_getattr_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
fprintf(f, " id %u", val);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] fs/select: add vmalloc fallback for select(2)
From: Vlastimil Babka @ 2016-09-22 17:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexander Viro, Andrew Morton, linux-fsdevel, linux-kernel,
linux-mm, Michal Hocko, netdev, Linux API, linux-man
In-Reply-To: <1474564068.23058.144.camel@edumazet-glaptop3.roam.corp.google.com>
On 09/22/2016 07:07 PM, Eric Dumazet wrote:
> On Thu, 2016-09-22 at 18:56 +0200, Vlastimil Babka wrote:
>> On 09/22/2016 06:49 PM, Eric Dumazet wrote:
>> > On Thu, 2016-09-22 at 18:43 +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.
>> >
>> > vmalloc() uses a vmap_area_lock spinlock, and TLB flushes.
>> >
>> > So I guess allowing vmalloc() being called from an innocent application
>> > doing a select() might be dangerous, especially if this select() happens
>> > thousands of time per second.
>>
>> Isn't seq_buf_alloc() similarly exposed? And ipc_alloc()?
>
> Possibly.
>
> We don't have a library function (attempting kmalloc(), fallback to
> vmalloc() presumably to avoid abuses, but I guess some patches were
> accepted without thinking about this.
So in the case of select() it seems like the memory we need 6 bits per file
descriptor, multiplied by the highest possible file descriptor (nfds) as passed
to the syscall. According to the man page of select:
EINVAL nfds is negative or exceeds the RLIMIT_NOFILE resource limit (see
getrlimit(2)).
The code actually seems to silently cap the value instead of returning EINVAL
though? (IIUC):
/* max_fds can increase, so grab it once to avoid race */
rcu_read_lock();
fdt = files_fdtable(current->files);
max_fds = fdt->max_fds;
rcu_read_unlock();
if (n > max_fds)
n = max_fds;
The default for this cap seems to be 1024 where I checked (again, IIUC, it's
what ulimit -n returns?). I wasn't able to change it to more than 2048, which
makes the bitmaps still below PAGE_SIZE.
So if I get that right, the system admin would have to allow really large
RLIMIT_NOFILE to even make vmalloc() possible here. So I don't see it as a large
concern?
Vlastimil
--
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][V2] cxgb4: fix signed wrap around when decrementing index idx
From: Colin King @ 2016-09-22 17:48 UTC (permalink / raw)
To: Hariprasad S, netdev; +Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Change predecrement compare to post decrement compare to avoid an
unsigned integer wrap-around comparison when decrementing idx in
the while loop.
For example, when idx is zero, the current situation will
predecrement idx in the while loop, wrapping idx to the maximum
signed integer and cause out of bounds reads on rxq_info->msix_tbl[idx].
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
index d12a73e..42a9c8d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
@@ -367,7 +367,7 @@ int request_msix_queue_irqs_uld(struct adapter *adap, unsigned int uld_type)
}
return 0;
unwind:
- while (--idx >= 0) {
+ while (idx-- > 0) {
bmap_idx = rxq_info->msix_tbl[idx];
free_msix_idx_in_bmap(adap, bmap_idx);
free_irq(adap->msix_info_ulds[bmap_idx].vec,
--
2.9.3
^ permalink raw reply related
* Re: [PATCH iproute2 net-next] tc: m_vlan: Add vlan modify action
From: Shmulik Ladkani @ 2016-09-22 17:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Jamal Hadi Salim, Jiri Pirko, Shmulik Ladkani
In-Reply-To: <20160922090504.3eaa5266@xeon-e3>
On Thu, 22 Sep 2016 09:05:04 -0700 Stephen Hemminger <stephen@networkplumber.org> wrote:
> 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?
No reason in particular, was probably code evolution, will amend, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] net: thunderx: Support for byte queue limits
From: Florian Fainelli @ 2016-09-22 17:41 UTC (permalink / raw)
To: sunil.kovvuri, netdev; +Cc: Sunil Goutham, linux-kernel, linux-arm-kernel
In-Reply-To: <1474535121-13958-3-git-send-email-sunil.kovvuri@gmail.com>
On 09/22/2016 02:05 AM, sunil.kovvuri@gmail.com wrote:
> From: Sunil Goutham <sgoutham@cavium.com>
>
> This patch adds support for byte queue limits
>
> Signed-off-by: Sunil Goutham <sgoutham@cavium.com>
Where is the code that calls netdev_tx_reset_queue()? This is needed in
the function that brings down the interface, did your test survive a
up/down/up sequence?
> ---
> drivers/net/ethernet/cavium/thunder/nicvf_main.c | 11 ++++++--
> drivers/net/ethernet/cavium/thunder/nicvf_queues.c | 30 ++++++++++++++--------
> 2 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> index 7d00162..453e3a0 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> @@ -516,7 +516,8 @@ static int nicvf_init_resources(struct nicvf *nic)
> static void nicvf_snd_pkt_handler(struct net_device *netdev,
> struct cmp_queue *cq,
> struct cqe_send_t *cqe_tx,
> - int cqe_type, int budget)
> + int cqe_type, int budget,
> + unsigned int *tx_pkts, unsigned int *tx_bytes)
> {
> struct sk_buff *skb = NULL;
> struct nicvf *nic = netdev_priv(netdev);
> @@ -547,6 +548,8 @@ static void nicvf_snd_pkt_handler(struct net_device *netdev,
> }
> nicvf_put_sq_desc(sq, hdr->subdesc_cnt + 1);
> prefetch(skb);
> + (*tx_pkts)++;
> + *tx_bytes += skb->len;
> napi_consume_skb(skb, budget);
> sq->skbuff[cqe_tx->sqe_ptr] = (u64)NULL;
> } else {
> @@ -662,6 +665,7 @@ static int nicvf_cq_intr_handler(struct net_device *netdev, u8 cq_idx,
> struct cmp_queue *cq = &qs->cq[cq_idx];
> struct cqe_rx_t *cq_desc;
> struct netdev_queue *txq;
> + unsigned int tx_pkts = 0, tx_bytes = 0;
>
> spin_lock_bh(&cq->lock);
> loop:
> @@ -701,7 +705,7 @@ loop:
> case CQE_TYPE_SEND:
> nicvf_snd_pkt_handler(netdev, cq,
> (void *)cq_desc, CQE_TYPE_SEND,
> - budget);
> + budget, &tx_pkts, &tx_bytes);
> tx_done++;
> break;
> case CQE_TYPE_INVALID:
> @@ -730,6 +734,9 @@ done:
> netdev = nic->pnicvf->netdev;
> txq = netdev_get_tx_queue(netdev,
> nicvf_netdev_qidx(nic, cq_idx));
> + if (tx_pkts)
> + netdev_tx_completed_queue(txq, tx_pkts, tx_bytes);
> +
> nic = nic->pnicvf;
> if (netif_tx_queue_stopped(txq) && netif_carrier_ok(netdev)) {
> netif_tx_start_queue(txq);
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> index 178c5c7..a4fc501 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> @@ -1082,6 +1082,24 @@ static inline void nicvf_sq_add_cqe_subdesc(struct snd_queue *sq, int qentry,
> imm->len = 1;
> }
>
> +static inline void nicvf_sq_doorbell(struct nicvf *nic, struct sk_buff *skb,
> + int sq_num, int desc_cnt)
> +{
> + struct netdev_queue *txq;
> +
> + txq = netdev_get_tx_queue(nic->pnicvf->netdev,
> + skb_get_queue_mapping(skb));
> +
> + netdev_tx_sent_queue(txq, skb->len);
> +
> + /* make sure all memory stores are done before ringing doorbell */
> + smp_wmb();
> +
> + /* Inform HW to xmit all TSO segments */
> + nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
> + sq_num, desc_cnt);
> +}
> +
> /* Segment a TSO packet into 'gso_size' segments and append
> * them to SQ for transfer
> */
> @@ -1141,12 +1159,8 @@ static int nicvf_sq_append_tso(struct nicvf *nic, struct snd_queue *sq,
> /* Save SKB in the last segment for freeing */
> sq->skbuff[hdr_qentry] = (u64)skb;
>
> - /* make sure all memory stores are done before ringing doorbell */
> - smp_wmb();
> + nicvf_sq_doorbell(nic, skb, sq_num, desc_cnt);
>
> - /* Inform HW to xmit all TSO segments */
> - nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
> - sq_num, desc_cnt);
> nic->drv_stats.tx_tso++;
> return 1;
> }
> @@ -1219,12 +1233,8 @@ doorbell:
> nicvf_sq_add_cqe_subdesc(sq, qentry, tso_sqe, skb);
> }
>
> - /* make sure all memory stores are done before ringing doorbell */
> - smp_wmb();
> + nicvf_sq_doorbell(nic, skb, sq_num, subdesc_cnt);
>
> - /* Inform HW to xmit new packet */
> - nicvf_queue_reg_write(nic, NIC_QSET_SQ_0_7_DOOR,
> - sq_num, subdesc_cnt);
> return 1;
>
> append_fail:
>
--
Florian
^ permalink raw reply
* Fwd: Re: nfs broken on Fedora-24, 32-bit?
From: Ben Greear @ 2016-09-22 17:35 UTC (permalink / raw)
To: netdev
In-Reply-To: <08ad07ee-72e2-b478-d70d-87b08c24452f@candelatech.com>
This is probably not an NFS specific issue, though I guess possibly it is.
Forwarding to netdev in case someone wants to take a look at it.
Thanks,
Ben
-------- Forwarded Message --------
Subject: Re: nfs broken on Fedora-24, 32-bit?
Date: Fri, 16 Sep 2016 16:31:51 -0700
From: Ben Greear <greearb@candelatech.com>
Organization: Candela Technologies
To: Trond Myklebust <trondmy@primarydata.com>
CC: List Linux NFS Mailing <linux-nfs@vger.kernel.org>
On 09/15/2016 04:06 PM, Ben Greear wrote:
> On 09/15/2016 04:00 PM, Trond Myklebust wrote:
>> Hi Ben,
>>
>>> On Sep 15, 2016, at 18:32, Ben Greear <greearb@candelatech.com> wrote:
>>>
>>> I have a Fedora-24 machine mounting an NFS server running Fedora-13 (kernel 2.6.34.9-69.fc13.x86_64).
>>>
>>> F24 machine has this in /etc/fstab:
>>>
>>> 192.168.100.3:/mnt/d2 /mnt/d2 nfs nfsvers=3 0 0
>>>
>>> When I copy a file from f24-32 to the F-13 machine, the file size is the same,
>>> but the file is corrupted on the file server. I see a different md5sum each time.
>>>
>>> Various other systems (F21, F19, etc) can all copy to the F13 machine fine.
>>>
>>> And, F24-64 machine can copy to the F13 machine fine.
>>>
>>> Anyone seen something similar?
>>
>> Do you know if the corruption is happening on the read()s or on the write()s? Do you, for instance get the same corruption if you copy from a local file on
>> the F-24 client to the server? ..or if you copy from a file on the server to a local directory on the F-24 client?
>>
>> Cheers
>> Trond
>>
>
> Seems to be a write issue:
>
> # This is the nfs server:
>
> [greearb@fs3 candela_cdrom.5.3.5]$ md5sum gua-f21-32
> ad4073fa8b806bb82b85a645e21f5e67 gua-f21-32
> [greearb@fs3 candela_cdrom.5.3.5]$ md5sum ../greearb/tmp/gua-f21-32
> 582bfea0cc8cc52aa38dc0f5048d0156 ../greearb/tmp/gua-f21-32
> [greearb@fs3 candela_cdrom.5.3.5]$
>
>
> # This is the v-f24-32 client:
>
> greearb@v-f24-32 ~]$ cp /mnt/d2/pub/candela_cdrom.5.3.5/gua-f21-32 ./
> [greearb@v-f24-32 ~]$ md5sum gua-f21-32
> ad4073fa8b806bb82b85a645e21f5e67 gua-f21-32
> [greearb@v-f24-32 ~]$ cp gua-f21-32 /mnt/d2/pub/greearb/tmp/
> [greearb@v-f24-32 ~]$ md5sum /mnt/d2/pub/greearb/tmp/gua-f21-32
> ad4073fa8b806bb82b85a645e21f5e67 /mnt/d2/pub/greearb/tmp/gua-f21-32
>
>
> Interesting that the client reads back the file it copied over as if it were correct, but
> it shows up wrong on the nfs server. Maybe it is just reading a local cache?
>
> Thanks,
> Ben
>
Here is some more info on this:
We can only reproduce this on virtual machines using the KVM infrastructure, and only
when we use the rtl8139 virtual hardware (in bridge mode). With the e1000 virtual hardware
we cannot reproduce the problem.
Also, multiple different nfs servers (including much newer kernels) all show the same
behaviour with this broken nfs client.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH net] net: rtnl_register in net_ns_init need rtnl_lock
From: Cong Wang @ 2016-09-22 17:20 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <4c1b9aba-e6e8-babc-1bbe-aef2bc0bca53@stressinduktion.org>
On Thu, Sep 22, 2016 at 6:41 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> 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?
But they are called during boot, why is it possible to have a parallel
reader/writer at that time?
^ 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