* Re: [PATCH net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
From: David Miller @ 2016-09-29 5:54 UTC (permalink / raw)
To: gwshan; +Cc: netdev, joel, yuvali, benh
In-Reply-To: <1475125395-8459-2-git-send-email-gwshan@linux.vnet.ibm.com>
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
Date: Thu, 29 Sep 2016 15:03:08 +1000
> This replaces the atomic access to NCSI channel's state with READ_ONCE()
> and WRITE_ONCE() to avoid the above build warning. We needn't hold the
> channel's lock when updating its state as well. No logical changes
> introduced.
I don't understand this.
If it's important to take the lock for the list add/del, then it must
be important to make the state change appear atomic wrt. that lock as
well.
Can parallel threads of control enter these functions which change the
state? If so, then you need to make the state changes under the lock.
In fact, you probably have to make the state tests under the locks as
well.
If not, please explain what prevents it from happening.
Thanks.
^ permalink raw reply
* net-next tree broken with CONFIG_NETFILTER_INGRESS=n
From: Marcel Holtmann @ 2016-09-29 6:23 UTC (permalink / raw)
To: Network Development
Hi Dave,
the net-next tree is broken since a few days now when CONFIG_NETFILTER_INGRESS=n is set.
CC net/netfilter/core.o
In file included from ./include/linux/linkage.h:4:0,
from ./include/linux/kernel.h:6,
from net/netfilter/core.c:10:
net/netfilter/core.c: In function ‘nf_set_hooks_head’:
net/netfilter/core.c:96:30: error: ‘struct net_device’ has no member named ‘nf_hooks_ingress’
rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
^
./include/linux/compiler.h:299:17: note: in definition of macro ‘WRITE_ONCE’
union { typeof(x) __val; char __c[1]; } __u = \
^
Regards
Marcel
^ permalink raw reply
* Re: [PATCH RFC 5/6] net: phy: Trigger state machine on state change and not polling.
From: Andrew Lunn @ 2016-09-29 7:08 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Vivien Didelot, netdev
In-Reply-To: <de3b7050-4346-1a7a-4a27-feb8bb8affc4@gmail.com>
On Wed, Sep 28, 2016 at 02:31:54PM -0700, Florian Fainelli wrote:
> On 09/28/2016 01:32 AM, Andrew Lunn wrote:
> > The phy_start() is used to indicate the PHY is now ready to do its
> > work. The state is changed, normally to PHY_UP which means that both
> > the MAC and the PHY are ready.
> >
> > If the phy driver is using polling, when the next poll happens, the
> > state machine notices the PHY is now in PHY_UP, and kicks off
> > auto-negotiation, if needed.
> >
> > If however, the PHY is using interrupts, there is no polling. The phy
> > is stuck in PHY_UP until the next interrupt comes along. And there is
> > no reason for the PHY to interrupt.
> >
> > Have phy_start() schedule the state machine to run, which both speeds
> > up the polling use case, and makes the interrupt use case actually
> > work.
> >
> > This problems exists whenever there is a state change which will not
> > cause an interrupt. Trigger the state machine in these cases,
> > e.g. phy_error().
> >
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>
> No particular objections, this should also fix this:
>
> http://lists.openwall.net/netdev/2016/05/17/147
Hi Florian
Yes, i was thinking it probably should have a fixes: tag and be a
separate patch. The hard part will be figuring out when this actually
broke, or does it go all the way back to when interrupt support was
added?
Andrew
^ permalink raw reply
* [PATCH] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
From: Maciej Żenczykowski @ 2016-09-29 7:33 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller
Cc: netdev, Erik Kline, Lorenzo Colitti
From: Maciej Żenczykowski <maze@google.com>
This is an effective no-op in terms of user observable behaviour.
By preventing the overwrite of non-null extra1/extra2 fields
in addrconf_sysctl() we can enable the use of proc_dointvec_minmax().
This allows us to eliminate the constant min/max (1..255) trampoline
function that is addrconf_sysctl_hop_limit().
This is nice because it simplifies the code, and allows future
sysctls with constant min/max limits to also not require trampolines.
We still can't eliminate the trampoline for mtu because it isn't
actually a constant (it depends on other tunables of the device)
and thus requires at-write-time logic to enforce range.
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/addrconf.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2f1f5d439788..8bd2d06eefe7 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5467,20 +5467,6 @@ int addrconf_sysctl_forward(struct ctl_table *ctl, int write,
}
static
-int addrconf_sysctl_hop_limit(struct ctl_table *ctl, int write,
- void __user *buffer, size_t *lenp, loff_t *ppos)
-{
- struct ctl_table lctl;
- int min_hl = 1, max_hl = 255;
-
- lctl = *ctl;
- lctl.extra1 = &min_hl;
- lctl.extra2 = &max_hl;
-
- return proc_dointvec_minmax(&lctl, write, buffer, lenp, ppos);
-}
-
-static
int addrconf_sysctl_mtu(struct ctl_table *ctl, int write,
void __user *buffer, size_t *lenp, loff_t *ppos)
{
@@ -5713,6 +5699,9 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
return ret;
}
+static const int one = 1;
+static const int two_five_five = 255;
+
static const struct ctl_table addrconf_sysctl[] = {
{
.procname = "forwarding",
@@ -5726,7 +5715,9 @@ static const struct ctl_table addrconf_sysctl[] = {
.data = &ipv6_devconf.hop_limit,
.maxlen = sizeof(int),
.mode = 0644,
- .proc_handler = addrconf_sysctl_hop_limit,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = (void *)&one,
+ .extra2 = (void *)&two_five_five,
},
{
.procname = "mtu",
@@ -6044,8 +6035,14 @@ static int __addrconf_sysctl_register(struct net *net, char *dev_name,
for (i = 0; table[i].data; i++) {
table[i].data += (char *)p - (char *)&ipv6_devconf;
- table[i].extra1 = idev; /* embedded; no ref */
- table[i].extra2 = net;
+ /* If one of these is already set, then it is not safe to
+ * overwrite either of them: this makes proc_dointvec_minmax
+ * usable.
+ */
+ if (!table[i].extra1 && !table[i].extra2) {
+ table[i].extra1 = idev; /* embedded; no ref */
+ table[i].extra2 = net;
+ }
}
snprintf(path, sizeof(path), "net/ipv6/conf/%s", dev_name);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-29 7:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475113378.28155.124.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
Hi Eric,
On Wed, 2016-09-28 at 18:42 -0700, Eric Dumazet wrote:
> On Wed, 2016-09-28 at 12:52 +0200, Paolo Abeni wrote:
>
> > +static void udp_rmem_release(struct sock *sk, int partial)
> > +{
> > + struct udp_sock *up = udp_sk(sk);
> > + int fwd, amt;
> > +
> > + if (partial && !udp_under_memory_pressure(sk))
> > + return;
> > +
> > + /* we can have concurrent release; if we catch any conflict
> > + * we let only one of them do the work
> > + */
> > + if (atomic_dec_if_positive(&up->can_reclaim) < 0)
> > + return;
> > +
> > + fwd = __udp_forward(up, atomic_read(&sk->sk_rmem_alloc));
> > + if (fwd < SK_MEM_QUANTUM + partial) {
> > + atomic_inc(&up->can_reclaim);
> > + return;
> > + }
> > +
> > + amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
> > + atomic_sub(amt, &up->mem_allocated);
> > + atomic_inc(&up->can_reclaim);
> > +
> > + __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> > + sk->sk_forward_alloc = fwd - amt;
> > +}
Thank you for reviewing this!
> This is racy...
Could you please elaborate?
> all these atomics make me nervous...
I'd like to drop some of them if possible.
atomic_inc(&up->can_reclaim);
could probably be replaced with atomic_set(&up->can_reclaim, 1) since we
don't have concurrent processes doing that and can_reclaim.counter is
known to be 0 at that point.
Performance wise the impact is minimal, since in normal condition we do
the reclaim only on socket shutdown.
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 v1] mlx4: remove unused fields
From: Tariq Toukan @ 2016-09-29 7:44 UTC (permalink / raw)
To: David Decotigny, Yishai Hadas, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: edumazet-hpIqsD4AKlfQT0dZR+AlfA, David Decotigny
In-Reply-To: <1475085604-101493-1-git-send-email-ddecotig-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 28/09/2016 9:00 PM, David Decotigny wrote:
> From: David Decotigny <decot-Ypc/8FJVVoBWk0Htik3J/w@public.gmane.org>
>
> This also can address following UBSAN warnings:
> [ 36.640343] ================================================================================
> [ 36.648772] UBSAN: Undefined behaviour in drivers/net/ethernet/mellanox/mlx4/fw.c:857:26
> [ 36.656853] shift exponent 64 is too large for 32-bit type 'int'
> [ 36.663348] ================================================================================
> [ 36.671783] ================================================================================
> [ 36.680213] UBSAN: Undefined behaviour in drivers/net/ethernet/mellanox/mlx4/fw.c:861:27
> [ 36.688297] shift exponent 35 is too large for 32-bit type 'int'
> [ 36.694702] ================================================================================
>
> Tested:
> reboot with UBSAN, no warning.
>
> Signed-off-by: David Decotigny <decot-Ypc/8FJVVoBWk0Htik3J/w@public.gmane.org>
> ---
> drivers/net/ethernet/mellanox/mlx4/fw.c | 4 ----
> drivers/net/ethernet/mellanox/mlx4/fw.h | 2 --
> 2 files changed, 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
> index 090bf81..f9cbc67 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/fw.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
> @@ -853,12 +853,8 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
> dev_cap->max_eqs = 1 << (field & 0xf);
> MLX4_GET(field, outbox, QUERY_DEV_CAP_RSVD_MTT_OFFSET);
> dev_cap->reserved_mtts = 1 << (field >> 4);
> - MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_MRW_SZ_OFFSET);
> - dev_cap->max_mrw_sz = 1 << field;
> MLX4_GET(field, outbox, QUERY_DEV_CAP_RSVD_MRW_OFFSET);
> dev_cap->reserved_mrws = 1 << (field & 0xf);
> - MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_MTT_SEG_OFFSET);
> - dev_cap->max_mtt_seg = 1 << (field & 0x3f);
> MLX4_GET(size, outbox, QUERY_DEV_CAP_NUM_SYS_EQ_OFFSET);
> dev_cap->num_sys_eqs = size & 0xfff;
> MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_REQ_QP_OFFSET);
> diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.h b/drivers/net/ethernet/mellanox/mlx4/fw.h
> index f11614f..5343a05 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/fw.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/fw.h
> @@ -80,9 +80,7 @@ struct mlx4_dev_cap {
> int max_eqs;
> int num_sys_eqs;
> int reserved_mtts;
> - int max_mrw_sz;
> int reserved_mrws;
> - int max_mtt_seg;
> int max_requester_per_qp;
> int max_responder_per_qp;
> int max_rdma_global;
Reviewed-by: Tariq Toukan <tariqt-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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/RFC 00/12] Programming Open vSwitch (-like) flows into hardware using SwitchDev
From: Simon Horman @ 2016-09-29 8:09 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Linux Netdev List, dev@openvswitch.org, Rony Efraim
In-Reply-To: <CAJ3xEMh0Wp6sXN1AxuZjnqycp6SkY732E4O9CRWip5Bydxuj2w@mail.gmail.com>
Hi Or,
On Wed, Sep 28, 2016 at 04:54:40PM +0300, Or Gerlitz wrote:
> On Wed, Sep 28, 2016 at 3:42 PM, Simon Horman
> <simon.horman@netronome.com> wrote:
>
> > A different approach, not implemented by this patch-set, is for user-space
> > to program flows into hardware by some other means, for example TC, and/or
> > the (kernel) datapath.
>
> Right, and we've submitted that code to the OVS community 24h ago [1].
>
> This was done along the feedback we've got for the last two years (since
> the LPC 2014 networking micro-conf). It allows offloading from
> multiple user-space
> applications through a single UAPI -- the TC one (currently we did
> flower, but the OVSD
> patch set can be extended to use whatever TC offloads are supported by
> the port driver,
> e.g U32, eBPF) and integration with 3rd party policy modules running
> in user-space.
>
> Lets hear people opinions and see where we go from now.
>
> > I believe that approach does not conflict with this one.
> > And there is some scope to share infrastructure in the kernel
>
> maybe, possibly
>
> We've having a talk in netdev 1.2 on offloading HW offloading of OVS
> and similar applications,
> I would encourage people to come and approach me and/or Rony Efraim
> from Mellanox before/after
> the talk to discuss that F2F, would love to get feedbacks, and also here...
Thanks for putting my post in context with the work you mention.
I am looking forward to some F2F discussions next week.
> Or.
>
> [1] pointers to patches implementing the 2nd approach
>
> cover-letter http://openvswitch.org/pipermail/dev/2016-September/079952.html
>
> patches
>
> https://patchwork.ozlabs.org/patch/675560/
> https://patchwork.ozlabs.org/patch/675567/
> https://patchwork.ozlabs.org/patch/675565/
> https://patchwork.ozlabs.org/patch/675559/
> https://patchwork.ozlabs.org/patch/675564/
> https://patchwork.ozlabs.org/patch/675563/
> https://patchwork.ozlabs.org/patch/675568/
> https://patchwork.ozlabs.org/patch/675566/
> https://patchwork.ozlabs.org/patch/675562/
>
> [2] http://www.netdevconf.org/1.2/session.html?rony-efraim-1
^ permalink raw reply
* [PATCH v3 net 2/2] net: skbuff: Limit skb_vlan_pop/push() to expect skb->data at mac header
From: Shmulik Ladkani @ 2016-09-29 9:10 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Jamal Hadi Salim, Shmulik Ladkani, Daniel Borkmann,
Pravin Shelar, Jiri Pirko
In-Reply-To: <1475140241-23586-1-git-send-email-shmulik.ladkani@gmail.com>
skb_vlan_pop/push were too generic, trying to support the cases where
skb->data is at mac header, and cases where skb->data is arbitrarily
elsewhere.
Supporting an arbitrary skb->data was complex and bogus:
- It failed to unwind skb->data to its original location post actual
pop/push.
(Also, semantic is not well defined for unwinding: If data was into
the eth header, need to use same offset from start; But if data was
at network header or beyond, need to adjust the original offset
according to the push/pull)
- It mangled the rcsum post actual push/pop, without taking into account
that the eth bytes might already have been pulled out of the csum.
Most callers (ovs, bpf) already had their skb->data at mac_header upon
invoking skb_vlan_pop/push.
Last caller that failed to do so (act_vlan) has been recently fixed.
Therefore, to simplify things, no longer support arbitrary skb->data
inputs for skb_vlan_pop/push().
skb->data is expected to be exactly at mac_header; WARN otherwise.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Pravin Shelar <pshelar@ovn.org>
Cc: Jiri Pirko <jiri@mellanox.com>
---
v3: Instead of correcting unwinding of skb->data in skb_vlan_pop/push,
just kill the support for arbitraray skb->data inputs, and assume
given skb->data always points at mac_header.
Fix act_vlan, the sole user not adehering to this assumption.
v2: Instead of reducing mac_len by 4 bytes, which was found incorrect,
fix the problem of wrong unwinding of 'skb->data'
net/core/skbuff.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3864b4b68f..8c38263cdf 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4478,13 +4478,18 @@ EXPORT_SYMBOL(skb_ensure_writable);
static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
{
struct vlan_hdr *vhdr;
- unsigned int offset = skb->data - skb_mac_header(skb);
+ int offset = skb->data - skb_mac_header(skb);
int err;
- __skb_push(skb, offset);
+ if (WARN_ONCE(offset,
+ "__skb_vlan_pop got skb with skb->data not at mac header (offset %d)\n",
+ offset)) {
+ return -EINVAL;
+ }
+
err = skb_ensure_writable(skb, VLAN_ETH_HLEN);
if (unlikely(err))
- goto pull;
+ return err;
skb_postpull_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
@@ -4501,12 +4506,13 @@ static int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci)
skb_set_network_header(skb, ETH_HLEN);
skb_reset_mac_len(skb);
-pull:
- __skb_pull(skb, offset);
return err;
}
+/* Pop a vlan tag either from hwaccel or from payload.
+ * Expects skb->data at mac header.
+ */
int skb_vlan_pop(struct sk_buff *skb)
{
u16 vlan_tci;
@@ -4541,29 +4547,30 @@ int skb_vlan_pop(struct sk_buff *skb)
}
EXPORT_SYMBOL(skb_vlan_pop);
+/* Push a vlan tag either into hwaccel or into payload (if hwaccel tag present).
+ * Expects skb->data at mac header.
+ */
int skb_vlan_push(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci)
{
if (skb_vlan_tag_present(skb)) {
- unsigned int offset = skb->data - skb_mac_header(skb);
+ int offset = skb->data - skb_mac_header(skb);
int err;
- /* __vlan_insert_tag expect skb->data pointing to mac header.
- * So change skb->data before calling it and change back to
- * original position later
- */
- __skb_push(skb, offset);
+ if (WARN_ONCE(offset,
+ "skb_vlan_push got skb with skb->data not at mac header (offset %d)\n",
+ offset)) {
+ return -EINVAL;
+ }
+
err = __vlan_insert_tag(skb, skb->vlan_proto,
skb_vlan_tag_get(skb));
- if (err) {
- __skb_pull(skb, offset);
+ if (err)
return err;
- }
skb->protocol = skb->vlan_proto;
skb->mac_len += VLAN_HLEN;
skb_postpush_rcsum(skb, skb->data + (2 * ETH_ALEN), VLAN_HLEN);
- __skb_pull(skb, offset);
}
__vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
return 0;
--
2.7.4
^ permalink raw reply related
* [PATCH v3 net 1/2] net/sched: act_vlan: Push skb->data to mac_header prior calling skb_vlan_*() functions
From: Shmulik Ladkani @ 2016-09-29 9:10 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Jamal Hadi Salim, Shmulik Ladkani, Daniel Borkmann,
Pravin Shelar, Jiri Pirko
Generic skb_vlan_push/skb_vlan_pop functions don't properly handle the
case where the input skb data pointer does not point at the mac header:
- They're doing push/pop, but fail to properly unwind data back to its
original location.
For example, in the skb_vlan_push case, any subsequent
'skb_push(skb, skb->mac_len)' calls make the skb->data point 4 bytes
BEFORE start of frame, leading to bogus frames that may be transmitted.
- They update rcsum per the added/removed 4 bytes tag.
Alas if data is originally after the vlan/eth headers, then these
bytes were already pulled out of the csum.
OTOH calling skb_vlan_push/skb_vlan_pop with skb->data at mac_header
present no issues.
act_vlan is the only caller to skb_vlan_*() that has skb->data pointing
at network header (upon ingress).
Other calles (ovs, bpf) already adjust skb->data at mac_header.
This patch fixes act_vlan to point to the mac_header prior calling
skb_vlan_*() functions, as other callers do.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Pravin Shelar <pshelar@ovn.org>
Cc: Jiri Pirko <jiri@mellanox.com>
---
net/sched/act_vlan.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 691409de3e..4ffc6c13a5 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -36,6 +36,12 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
bstats_update(&v->tcf_bstats, skb);
action = v->tcf_action;
+ /* Ensure 'data' points at mac_header prior calling vlan manipulating
+ * functions.
+ */
+ if (skb_at_tc_ingress(skb))
+ skb_push_rcsum(skb, skb->mac_len);
+
switch (v->tcfv_action) {
case TCA_VLAN_ACT_POP:
err = skb_vlan_pop(skb);
@@ -57,6 +63,9 @@ drop:
action = TC_ACT_SHOT;
v->tcf_qstats.drops++;
unlock:
+ if (skb_at_tc_ingress(skb))
+ skb_pull_rcsum(skb, skb->mac_len);
+
spin_unlock(&v->tcf_lock);
return action;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] ipv6 addrconf: remove addrconf_sysctl_hop_limit()
From: Erik Kline @ 2016-09-29 9:15 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: Maciej Żenczykowski, David S . Miller, netdev,
Lorenzo Colitti
In-Reply-To: <1475134423-23635-1-git-send-email-zenczykowski@gmail.com>
Seems fine to me.
Acked-by: Erik Kline <ek@google.com>
^ permalink raw reply
* Re: [PATCH v3 net 1/2] net/sched: act_vlan: Push skb->data to mac_header prior calling skb_vlan_*() functions
From: Shmulik Ladkani @ 2016-09-29 9:20 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Jamal Hadi Salim, Daniel Borkmann, Pravin Shelar,
Jiri Pirko
In-Reply-To: <1475140241-23586-1-git-send-email-shmulik.ladkani@gmail.com>
David,
On Thu, 29 Sep 2016 12:10:40 +0300 Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote:
> This patch fixes act_vlan to point to the mac_header prior calling
> skb_vlan_*() functions, as other callers do.
>
This 1/2 patch fixes the problem detailed in [1] for act_vlan,
last known caller of skb_vlan_*() with skb->data not at mac_header.
I think it's a good candidate for -stable; it fixes the observed bug and
it is rather focused.
Subsequent 2/2 patch hermetically seals the future possibility that one
might call skb_vlan_*() with skb->data not at mac_header.
This might go to stable as well, but not strictly required.
Thanks,
Shmulik
[1] https://patchwork.ozlabs.org/patch/676111/
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] udp: implement memory accounting helpers
From: Paolo Abeni @ 2016-09-29 9:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, James Morris,
Trond Myklebust, Alexander Duyck, Daniel Borkmann, Eric Dumazet,
Tom Herbert, Hannes Frederic Sowa, Edward Cree,
linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475113378.28155.124.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Wed, 2016-09-28 at 18:42 -0700, Eric Dumazet wrote:
> On Wed, 2016-09-28 at 12:52 +0200, Paolo Abeni wrote:
>
> > +static void udp_rmem_release(struct sock *sk, int partial)
> > +{
> > + struct udp_sock *up = udp_sk(sk);
> > + int fwd, amt;
> > +
> > + if (partial && !udp_under_memory_pressure(sk))
> > + return;
> > +
> > + /* we can have concurrent release; if we catch any conflict
> > + * we let only one of them do the work
> > + */
> > + if (atomic_dec_if_positive(&up->can_reclaim) < 0)
> > + return;
> > +
> > + fwd = __udp_forward(up, atomic_read(&sk->sk_rmem_alloc));
> > + if (fwd < SK_MEM_QUANTUM + partial) {
> > + atomic_inc(&up->can_reclaim);
> > + return;
> > + }
> > +
> > + amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
> > + atomic_sub(amt, &up->mem_allocated);
> > + atomic_inc(&up->can_reclaim);
> > +
> > + __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> > + sk->sk_forward_alloc = fwd - amt;
> > +}
>
>
> This is racy... all these atomics make me nervous...
Ah, perhaps I got it: if we have a concurrent memory scheduling, we
could end up with a value of mem_allocated below the real need.
That mismatch will not drift: at worst we can end up with mem_allocated
being single SK_MEM_QUANTUM below what is strictly needed.
A possible alternative could be:
static void udp_rmem_release(struct sock *sk, int partial)
{
struct udp_sock *up = udp_sk(sk);
int fwd, amt, alloc_old, alloc;
if (partial && !udp_under_memory_pressure(sk))
return;
alloc = atomic_read(&up->mem_allocated);
fwd = alloc - atomic_read(&sk->sk_rmem_alloc);
if (fwd < SK_MEM_QUANTUM + partial)
return;
amt = (fwd - partial) & ~(SK_MEM_QUANTUM - 1);
alloc_old = atomic_cmpxchg(&up->mem_allocated, alloc, alloc - amt);
/* if a concurrent update is detected, just do nothing; if said update
* is due to another memory release, that release take care of
* reclaiming the memory for us, too.
* Otherwise we will be able to release on later dequeue, since
* we will eventually stop colliding with the writer when it will
* consume all the fwd allocated memory
*/
if (alloc_old != alloc)
return;
__sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
sk->sk_forward_alloc = fwd - amt;
}
which is even more lazy in reclaiming but should never underestimate the
needed forward allocation, and under pressure should eventually free the
needed memory.
--
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] ipv6 addrconf: implement RFC7559 router solicitation backoff
From: Erik Kline @ 2016-09-29 9:56 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: Hannes Frederic Sowa, David S . Miller, Linux NetDev,
Lorenzo Colitti, Hideaki Yoshifuji
In-Reply-To: <CANP3RGe4jMRhepM5v2Dg4Kq3Rxj=tA7WJEMcD_83DLYcEgujMA@mail.gmail.com>
Passes my local unittest for this behaviour.
Acked-by: Erik Kline <ek@google.com>
^ permalink raw reply
* Re: How to submit potential patch in linux kernel
From: Andrew Lunn @ 2016-09-29 10:02 UTC (permalink / raw)
To: Shyam Saini; +Cc: netdev
In-Reply-To: <20160928230229.GA15734@shyam>
On Thu, Sep 29, 2016 at 04:32:29AM +0530, Shyam Saini wrote:
> Hi everyone,
>
> I'm Shyam, final year undergraduate student. I wanted to know how one can
> submit potential linux kernel patch in networking subsystem.
Documentation/SubmittingPatches is a good starting point.
Then post a patch and we will help you learn the rest of the process.
Andrew
^ permalink raw reply
* company
From: Samsung @ 2016-09-29 10:05 UTC (permalink / raw)
To: Net-info16
[-- Attachment #1: Type: text/plain, Size: 3 bytes --]
[-- Attachment #2: 1-shyv5.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 517062 bytes --]
^ permalink raw reply
* Re: How to submit potential patch in linux kernel
From: Daniel Baluta @ 2016-09-29 10:15 UTC (permalink / raw)
To: Shyam Saini; +Cc: netdev@vger.kernel.org, Andrew Lunn
In-Reply-To: <20160929100249.GD7234@lunn.ch>
Hi Shyam,
On Thu, Sep 29, 2016 at 1:02 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Thu, Sep 29, 2016 at 04:32:29AM +0530, Shyam Saini wrote:
>> Hi everyone,
>>
>> I'm Shyam, final year undergraduate student. I wanted to know how one can
>> submit potential linux kernel patch in networking subsystem.
>
> Documentation/SubmittingPatches is a good starting point.
>
> Then post a patch and we will help you learn the rest of the process.
You can have a look at https://kernelnewbies.org/FirstKernelPatch.
There is also this presentation from Greg:
https://www.youtube.com/watch?v=LLBrBBImJt4
thanks,
Daniel.
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Shmulik Ladkani @ 2016-09-29 10:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, Jamal Hadi Salim, WANG Cong, Eric Dumazet,
Daniel Borkmann, Florian Westphal, netdev
In-Reply-To: <1475011633.28155.68.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
On Tue, 27 Sep 2016 14:27:13 -0700 Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Since this runs lockless, another cpu might change m->tcfm_eaction in
> the middle, and you could call dev_queue_xmit(skb2) while the skb2 was
> prepared for the opposite action.
Well, seem members of 'struct tcf_mirred' are out of sync wrt to each
other, even in existing code, regadless this patch:
- 'tcfm_dev' may be assigned, but 'tcfm_ok_push' not yet updated,
may result in skb_push_rcsum being called/not called
- 'tcfm_eaction' is changed, in between "mirror is always swallowed" to
the final 'out:' label,
may result in wrong tc_verd assigned (or lack of assignment)
Seems the whole "params" need be rcu_dereferenced, like in
tunnel_key_act, or like your suggestion in
https://patchwork.ozlabs.org/patch/667680/.
I'm gonna fix the new problem you pointed out, by reading-once
'tcfm_eaction' early (right when tcfm_dev is dereferenced) knowing this
is just "keeping things as is wrt running lockless", without introducing
any new non-coherent code.
Thanks,
Shmulik
^ permalink raw reply
* [PATCH v3 net-next 0/4] act_mirred: Ingress actions support
From: Shmulik Ladkani @ 2016-09-29 11:03 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
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.
v3:
in 4/4, addressed non coherency due to reading m->tcfm_eaction multiple
times, as spotted by Eric Dumazet
v2:
in 1/4, declare tcfm_mac_header_xmit as bool instead of int
Shmulik Ladkani (4):
net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit and
make it a bool
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 | 84 ++++++++++++++++------
7 files changed, 73 insertions(+), 29 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v3 net-next 1/4] net/sched: act_mirred: Rename tcfm_ok_push to tcfm_mac_header_xmit and make it a bool
From: Shmulik Ladkani @ 2016-09-29 11:03 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
netdev, Shmulik Ladkani
In-Reply-To: <1475147012-15538-1-git-send-email-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 (and use a bool instead of int).
This allows to decouple the attribute from the action to be taken.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
v2: declare tcfm_mac_header_xmit as bool instead of int
include/net/tc_act/tc_mirred.h | 2 +-
net/sched/act_mirred.c | 11 ++++++-----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h
index 62770add15..95431092c4 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;
+ bool 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 667dc382df..16e17a887b 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -60,11 +60,12 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
{
struct tc_action_net *tn = net_generic(net, mirred_net_id);
struct nlattr *tb[TCA_MIRRED_MAX + 1];
+ bool mac_header_xmit = false;
struct tc_mirred *parm;
struct tcf_mirred *m;
struct net_device *dev;
- int ret, ok_push = 0;
bool exists = false;
+ int ret;
if (nla == NULL)
return -EINVAL;
@@ -102,10 +103,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 = false;
break;
default:
- ok_push = 1;
+ mac_header_xmit = true;
break;
}
} else {
@@ -136,7 +137,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 +182,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);
}
--
2.7.4
^ permalink raw reply related
* [PATCH v3 net-next 2/4] net/sched: act_mirred: Refactor detection whether dev needs xmit at mac header
From: Shmulik Ladkani @ 2016-09-29 11:03 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
netdev, Shmulik Ladkani
In-Reply-To: <1475147012-15538-1-git-send-email-shmulik.ladkani@gmail.com>
Move detection logic that tests whether device expects skb data to point
at 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 16e17a887b..69dcce8c75 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 bool 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 false;
+ }
+ return true;
+}
+
static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct nlattr *est, struct tc_action **a, int ovr,
int bind)
@@ -96,19 +110,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 = false;
- break;
- default:
- mac_header_xmit = true;
- break;
- }
+ mac_header_xmit = dev_is_mac_header_xmit(dev);
} else {
dev = NULL;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v3 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-29 11:03 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
netdev, Shmulik Ladkani, Hariprasad S, Jeff Kirsher,
Saeed Mahameed, Jiri Pirko, Ido Schimmel, Jakub Kicinski
In-Reply-To: <1475147012-15538-1-git-send-email-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 49d2debb33..52af62e0ec 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 a244d9a672..784b0b98ab 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -8410,7 +8410,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 a350b7171e..957a464489 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -404,7 +404,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 fd74d1064f..6a4f9c4664 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 8acfb631a0..cfed40c0e3 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 95431092c4..604bc31e23 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)
--
2.7.4
^ permalink raw reply related
* [PATCH v3 net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: Shmulik Ladkani @ 2016-09-29 11:03 UTC (permalink / raw)
To: David Miller
Cc: Jamal Hadi Salim, WANG Cong, Eric Dumazet, Daniel Borkmann,
netdev, Shmulik Ladkani, Eric Dumazet
In-Reply-To: <1475147012-15538-1-git-send-email-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>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
v3: Addressed non coherency due to reading m->tcfm_eaction multiple times,
as spotted by Eric Dumazet
net/sched/act_mirred.c | 51 ++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 69dcce8c75..22dcfd68e6 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);
@@ -97,6 +116,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)
@@ -156,15 +177,20 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
struct tcf_result *res)
{
struct tcf_mirred *m = to_mirred(a);
+ bool m_mac_header_xmit;
struct net_device *dev;
struct sk_buff *skb2;
- int retval, err;
+ int retval, err = 0;
+ int m_eaction;
+ int mac_len;
u32 at;
tcf_lastuse_update(&m->tcf_tm);
bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
rcu_read_lock();
+ m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);
+ m_eaction = READ_ONCE(m->tcfm_eaction);
retval = READ_ONCE(m->tcf_action);
dev = rcu_dereference(m->tcfm_dev);
if (unlikely(!dev)) {
@@ -183,23 +209,36 @@ 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_eaction) && m_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_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_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_eaction))
retval = TC_ACT_SHOT;
}
rcu_read_unlock();
--
2.7.4
^ permalink raw reply related
* Re: [RFC] net: store port/representative id in metadata_dst
From: Jakub Kicinski @ 2016-09-29 11:10 UTC (permalink / raw)
To: John Fastabend
Cc: Jakub Kicinski, Samudrala, Sridhar, Jiri Benc, Jiri Pirko, netdev,
Thomas Graf, Roopa Prabhu, ogerlitz, ast, daniel, simon.horman,
Paolo Abeni, Pravin B Shelar, hannes
In-Reply-To: <57E59CA8.30007@gmail.com>
On Fri, 23 Sep 2016 14:20:40 -0700, John Fastabend wrote:
> On 16-09-23 01:45 PM, Jakub Kicinski wrote:
> > On Fri, 23 Sep 2016 13:25:10 -0700, John Fastabend wrote:
> >> On 16-09-23 01:17 PM, Jakub Kicinski wrote:
> >>> On Fri, 23 Sep 2016 10:22:59 -0700, Samudrala, Sridhar wrote:
> >>>> On 9/23/2016 8:29 AM, Jakub Kicinski wrote:
> >> [...]
> >> [...]
> >>>>
> >>>> The 'accel' parameter in dev_queue_xmit_accel() is currently only passed
> >>>> to ndo_select_queue() via netdev_pick_tx() and is used to select the tx
> >>>> queue.
> >>>> Also, it is not passed all the way to the driver specific xmit routine.
> >>>> Doesn't it require
> >>>> changing all the driver xmit routines if we want to pass this parameter?
> >>>>
> >> [...]
> >>>>
> >>>> Yes. The VFPR netdevs don't have any HW queues associated with them and
> >>>> we would like
> >>>> to use the PF queues for the xmit.
> >>>> I was also looking into some way of passing the port id via skb
> >>>> parameter to the
> >>>> dev_queue_xmit() call so that the PF xmit routine can do a directed
> >>>> transmit to a specifc VF.
> >>>> Is skb->cb an option to pass this info?
> >>>> dst_metadata approach would work too if it is acceptable.
> >>>
> >>> I don't think we can trust skb->cb to be set to anything meaningful
> >>> when the skb is received by the lower device.
> >>
> >> Agreed. I wouldn't recommend using skb->cb. How about passing it through
> >> dev_queue_xmit_accel() through to the driver?
> >>
> >> If you pass the metadata through the dev_queue_xmit_accel() handle tx
> >> queue selection would work using normal mechanisms (xps, select_queue,
> >> cls hook, etc.). If you wanted to pick some specific queue based on
> >> policy the policy could be loaded into one of those hooks.
> >
> > Do you mean without extending how accel is handled by
> > dev_queue_xmit_accel() today? If my goal is to not have extra HW
> > queues then I don't see how I could mux in the lower dev without extra
> > locking (as I tried to explain two emails ago). Sorry for being slow
> > here :(
> >
>
> Not slow here I think I was overly optimistic...
>
> Yeh let me try this, roughly the current flow is,
>
> dev_queue_xmit_accel(struct sk_buff *skb, void *accel_priv);
> __dev_queue_xmit(skb, accel_priv);
> netdev_pick_tx(dev, skb, accel_priv);
> ndo_select_queue(dev, skb, accel_priv, ...);
> [...]
> q->enqueue();
> [...]
> dev_hard_start_xmit();
> [...]
> <driver code here>
>
> So in this flow the VFR netdev driver handles its xmit routine by
> calling dev_queue_xmit_accel after setting skb->dev to the physical
> device and passing a cookie via accel that the select_queue() routine
> can use to pick a tx queue. The rest of the stack q->enqueue() and
> friends will ensure that locking and qdisc is handled correctly.
>
> But accel_priv was lost at queue selection and so its not being passed
> down to the driver so no way to set your descriptor bits or whatever
> needed to push to the VF. I was sort of thinking we could map it from
> the select_queue routine but I can't figure out how to do that either.
>
> The metadata idea doesn't seem that bad now that I've spent some more
> time going through it. Either that or hijack some field in the skb but
> I think that might be worse than the proposal here.
>
> I'm trying to think up some other alternative now and will let you know
> if I think of anything clever but got nothing at the moment.
Cool, I'm happy to discuss this further at netdev but it seems like
there is no strong opposition so far?
FWIW in the example I gave I didn't do refcounting on the dst but I
think that's incorrect since we don't have control over lifetime of
redirected/stolen skbs.
^ permalink raw reply
* RE: [PATCH 3/3] net: fec: align IP header in hardware
From: David Laight @ 2016-09-29 11:07 UTC (permalink / raw)
To: 'Eric Nelson', netdev@vger.kernel.org
Cc: linux@arm.linux.org.uk, andrew@lunn.ch, fugang.duan@nxp.com,
otavio@ossystems.com.br, edumazet@google.com,
troy.kisky@boundarydevices.com, davem@davemloft.net,
u.kleine-koenig@pengutronix.de
In-Reply-To: <5cf173c4-84e0-f309-f356-35b114cc166e@nelint.com>
From: Eric Nelson
> Sent: 28 September 2016 18:15
> On 09/28/2016 09:42 AM, David Laight wrote:
> > From: Eric Nelson
> >> Sent: 26 September 2016 19:40
> >> Hi David,
> >>
> >> On 09/26/2016 02:26 AM, David Laight wrote:
> >>> From: Eric Nelson
> >>>> Sent: 24 September 2016 15:42
> >>>> The FEC receive accelerator (RACC) supports shifting the data payload of
> >>>> received packets by 16-bits, which aligns the payload (IP header) on a
> >>>> 4-byte boundary, which is, if not required, at least strongly suggested
> >>>> by the Linux networking layer.
> >>> ...
> >>>> + /* align IP header */
> >>>> + val |= FEC_RACC_SHIFT16;
> >>>
> >>> I can't help feeling that there needs to be corresponding
> >>> changes to increase the buffer size by 2 (maybe for large mtu)
> >>> and to discard two bytes from the frame length.
> >>>
> >>
> >> In the normal case, the fec driver over-allocates all receive packets to
> >> be of size FEC_ENET_RX_FRSIZE (2048) minus the value of rx_align,
> >> which is either 0x0f (ARM) or 0x03 (PPC).
> >>
> >> If the frame length is less than rx_copybreak (typically 256), then
> >> the frame length from the receive buffer descriptor is used to
> >> control the allocation size for a copied buffer, and this will include
> >> the two bytes of padding if RACC_SHIFT16 is set.
> >>
> >>> If probably ought to be predicated on NET_IP_ALIGN as well.
> >>>
> >> Can you elaborate?
> >
> > From reading this it seems that the effect of FEC_RACC_SHIFT16 is to
> > add two bytes of 'junk' to the start of every receive frame.
> >
>
> That's right. Two bytes of junk between the MAC header and the
> IP header.
>
> > In the 'copybreak' case the new skb would need to be 2 bytes shorter
> > than the length reported by the hardware, and the data copied from
> > 2 bytes into the dma buffer.
> >
>
> As it stands, the skb allocated by the copybreak routine will include
> the two bytes of padding, and the call to skb_pull_inline will ignore
> them.
Ok, I didn't see that call being added by this patch.
> > The extra 2 bytes also mean the that maximum mtu that can be received
> > into a buffer is two bytes less.
> >
>
> Right, but I think the max is already high enough that this isn't a
> problem.
>
> > If someone sets the mtu to (say) 9k for jumbo frames this might matter.
> > Even with fixed 2048 byte buffers it reduces the maximum value the mtu
> > can be set to by 2.
> >
>
> As far as I can tell, the fec driver doesn't support jumbo frames, and
> the max frame length is currently hard-coded at PKT_MAXBUF_SIZE (1522).
>
> This is well within the 2048-byte allocation, even with optional headers
> for VLAN etc.
Hmm...
That (probably) means all the skb the driver allocates are actually 4k.
It would be much better to reduce the size so that the entire skb
(with packet buffer) is less than 2k.
> > Now if NET_IP_ALIGN is zero then it is fine for the rx frame to start
> > on a 4n boundary, and the skb are likely to be allocated that way.
> > In this case you don't want to extra two bytes of 'junk'.
> >
> NET_IP_ALIGN is defaulting to 2 by the conditional in skbuff.h
Even though it is always currently set is isn't really ideal to have
a driver that breaks if it isn't set.
This could easily happen at some point in the future if the ethernet
logic is put with a different cpu.
> > OTOH if NET_IP_ALIGN is 2 then you need to 'fiddle' things so that
> > the data is dma'd to offset -2 in the skb and then ensure that the
> > end of frame is set correctly.
> >
>
> That's what the RACC SHIFT16 bit does.
No, that causes the ethernet controller to add 2 bytes to the frame.
You then need to change the dma target address to match.
Otherwise if a new version of the silicon stops ignoring the low
address with the frame will be misaligned in the buffer.
The receive frame length will also (probably) be 2 larger than the
actual frame - so you need to set the end point correctly as well.
IP will probably ignore the 2 bytes of pad I think you are generating.
> The FEC hardware isn't capable of DMA'ing to an un-aligned address.
> On ARM, it requires 64-bit alignment, but suggests 128-bit alignment.
>
> On other (PPC?) architectures, it requires 32-bit alignment. This is
> handled by the rx_align field.
That isn't entirely relevant.
If the kernel is being built with NET_IP_ALIGN set to 0 you should
align the destination mac address on a 4n boundary
(Or rather the skb are likely to be allocated that way).
If it causes misaligned memory reads later on that is a different problem.
The MAC driver has aligned the frames as it was told to.
David
^ permalink raw reply
* Re: [PATCH net-next 1/8] net/ncsi: Avoid unused-value build warning from ia64-linux-gcc
From: Gavin Shan @ 2016-09-29 11:40 UTC (permalink / raw)
To: David Miller; +Cc: gwshan, netdev, joel, yuvali, benh
In-Reply-To: <20160929.015404.76401694814645494.davem@davemloft.net>
On Thu, Sep 29, 2016 at 01:54:04AM -0400, David Miller wrote:
>From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>Date: Thu, 29 Sep 2016 15:03:08 +1000
>
>> This replaces the atomic access to NCSI channel's state with READ_ONCE()
>> and WRITE_ONCE() to avoid the above build warning. We needn't hold the
>> channel's lock when updating its state as well. No logical changes
>> introduced.
>
>I don't understand this.
>
>If it's important to take the lock for the list add/del, then it must
>be important to make the state change appear atomic wrt. that lock as
>well.
>
>Can parallel threads of control enter these functions which change the
>state? If so, then you need to make the state changes under the lock.
>In fact, you probably have to make the state tests under the locks as
>well.
>
>If not, please explain what prevents it from happening.
>
Dave, thanks for your comments. I think it's occasionally working on
AST2400 and AST2500 platforms. It's reasonable to grab the lock before
fetching or updating the NCSI channel's state. Adding and removing the
channel from the list also need taking the lock as well. I will modify
the code accordingly in next revision.
AST2400/AST2500 has single CPU. The channel's state (and the linked
list) are changed in softirq context (packet Rx handler or timer),
meaning they are not accessed in parallel mode. However, NCSI stack
cannot make assumption to be run on single CPU platforms only. So
yes, we need the lock to protect them.
Thanks,
Gavin
^ 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