* [PATCH 2/2] net: stmmac: Fix crash observed if PHY does not support EEE
From: Jon Hunter @ 2019-06-26 10:23 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu
Cc: netdev, linux-kernel, linux-tegra, Jon Hunter
In-Reply-To: <20190626102322.18821-1-jonathanh@nvidia.com>
If the PHY does not support EEE mode, then a crash is observed when the
ethernet interface is enabled. The crash occurs, because if the PHY does
not support EEE, then although the EEE timer is never configured, it is
still marked as enabled and so the stmmac ethernet driver is still
trying to update the timer by calling mod_timer(). This triggers a BUG()
in the mod_timer() because we are trying to update a timer when there is
no callback function set because timer_setup() was never called for this
timer.
The problem is caused because we return true from the function
stmmac_eee_init(), marking the EEE timer as enabled, even when we have
not configured the EEE timer. Fix this by ensuring that we return false
if the PHY does not support EEE and hence, 'eee_active' is not set.
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6c6c6ec3c781..8f5ebd51859e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -398,10 +398,12 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
mutex_lock(&priv->lock);
/* Check if it needs to be deactivated */
- if (!priv->eee_active && priv->eee_enabled) {
- netdev_dbg(priv->dev, "disable EEE\n");
- del_timer_sync(&priv->eee_ctrl_timer);
- stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ if (!priv->eee_active) {
+ if (priv->eee_enabled) {
+ netdev_dbg(priv->dev, "disable EEE\n");
+ del_timer_sync(&priv->eee_ctrl_timer);
+ stmmac_set_eee_timer(priv, priv->hw, 0, tx_lpi_timer);
+ }
mutex_unlock(&priv->lock);
return false;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net] ipv4: fix suspicious RCU usage in fib_dump_info_fnhe()
From: Stefano Brivio @ 2019-06-26 10:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, David Ahern, syzbot
In-Reply-To: <20190626100450.217106-1-edumazet@google.com>
On Wed, 26 Jun 2019 03:04:50 -0700
Eric Dumazet <edumazet@google.com> wrote:
> sysbot reported that we lack appropriate rcu_read_lock()
> protection in fib_dump_info_fnhe()
Thanks for fixing this.
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 6aee412a68bdd3c24a6a0eb9883e04b7a83998e0..59670fafcd2612b94c237cbe30109adb196cf3f0 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2872,12 +2872,13 @@ int fib_dump_info_fnhe(struct sk_buff *skb, struct netlink_callback *cb,
> if (nhc->nhc_flags & RTNH_F_DEAD)
> continue;
>
> + rcu_read_lock();
> bucket = rcu_dereference(nhc->nhc_exceptions);
> - if (!bucket)
> - continue;
> -
> - err = fnhe_dump_bucket(net, skb, cb, table_id, bucket, genid,
> - fa_index, fa_start);
> + err = 0;
Could you perhaps move declaration and initialisation of 'err' outside
the block while at it? It looks a bit more readable at this point.
> + if (bucket)
> + err = fnhe_dump_bucket(net, skb, cb, table_id, bucket,
> + genid, fa_index, fa_start);
> + rcu_read_unlock();
> if (err)
> return err;
> }
Either way,
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply
* Re: [PATCH nf-next v2 2/2] netfilter: nft_meta: Add NFT_META_BRI_VLAN support
From: Pablo Neira Ayuso @ 2019-06-26 10:29 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <1560993460-25569-2-git-send-email-wenxu@ucloud.cn>
[-- Attachment #1: Type: text/plain, Size: 530 bytes --]
Could you add a NFT_META_BRI_VLAN_PROTO? Similar to patch 1/2, to
retrieve p->br->vlan_proto.
Then, add a generic way to set the vlan metadata. I'm attaching an
incomplete patch, so there is something like:
meta vlan set 0x88a8:20
to set q-in-q.
we could also add a shortcut for simple vlan case (no q-in-q), ie.
assuming protocol is 0x8100:
meta vlan set 20
Does this make sense to you?
And we have a way to set the meta vlan information from ingress to
then, which is something I also need here.
Thanks.
[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 2341 bytes --]
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 8859535031e2..6ef2cc42924c 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -796,6 +796,7 @@ enum nft_exthdr_attributes {
* @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_PVID: packet input bridge port pvid
+ * @NFT_META_VLAN: packet vlan metadata
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -827,6 +828,7 @@ enum nft_meta_keys {
NFT_META_IIFKIND,
NFT_META_OIFKIND,
NFT_META_BRI_PVID,
+ NFT_META_VLAN,
};
/**
@@ -893,12 +895,14 @@ enum nft_hash_attributes {
* @NFTA_META_DREG: destination register (NLA_U32)
* @NFTA_META_KEY: meta data item to load (NLA_U32: nft_meta_keys)
* @NFTA_META_SREG: source register (NLA_U32)
+ * @NFTA_META_SREG2: source register (NLA_U32)
*/
enum nft_meta_attributes {
NFTA_META_UNSPEC,
NFTA_META_DREG,
NFTA_META_KEY,
NFTA_META_SREG,
+ NFTA_META_SREG2,
__NFTA_META_MAX
};
#define NFTA_META_MAX (__NFTA_META_MAX - 1)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 4f8116de70f8..dbbad7319183 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -28,7 +28,10 @@ struct nft_meta {
enum nft_meta_keys key:8;
union {
enum nft_registers dreg:8;
- enum nft_registers sreg:8;
+ struct {
+ enum nft_registers sreg:8;
+ enum nft_registers sreg2:8;
+ };
};
};
@@ -304,6 +307,17 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
skb->secmark = value;
break;
#endif
+ case NFT_META_VLAN: {
+ u32 *sreg2 = ®s->data[meta->sreg2];
+ __be16 vlan_proto;
+ u16 vlan_tci;
+
+ vlan_tci = nft_reg_load16(sreg);
+ vlan_proto = nft_reg_load16(sreg2);
+
+ __vlan_hwaccel_put_tag(skb, vlan_proto, vlan_tci);
+ break;
+ }
default:
WARN_ON(1);
}
@@ -474,6 +488,13 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
case NFT_META_PKTTYPE:
len = sizeof(u8);
break;
+ case NFT_META_VLAN:
+ len = sizeof(u16);
+ priv->sreg2 = nft_parse_register(tb[NFTA_META_SREG2]);
+ err = nft_validate_register_load(priv->sreg2, len);
+ if (err < 0)
+ return err;
+ break;
default:
return -EOPNOTSUPP;
}
^ permalink raw reply related
* Re: [PATCH nf-next v2 1/2] netfilter: nft_meta: add NFT_META_BRI_PVID support
From: Pablo Neira Ayuso @ 2019-06-26 10:30 UTC (permalink / raw)
To: wenxu; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <20190626090116.3qjmlnd5egeifozq@salvia>
On Wed, Jun 26, 2019 at 11:01:16AM +0200, Pablo Neira Ayuso wrote:
> On Thu, Jun 20, 2019 at 09:17:39AM +0800, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.cn>
> >
> > nft add table bridge firewall
> > nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
> > nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
> >
> > As above set the bridge port with pvid, the received packet don't contain
> > the vlan tag which means the packet should belong to vlan 200 through pvid.
> > With this pacth user can get the pvid of bridge ports.
> >
> > So add the following rule for as the first rule in the chain of zones.
> >
> > nft add rule bridge firewall zones counter meta brvlan set meta brpvid
>
> Applied, thanks.
https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git/commit/?id=da4f10a4265b109fbdb77d5995236e0843e4a26d
This patch is applied.
2/2 is not, until we finish a bit of discussion on where to go.
Thanks.
^ permalink raw reply
* [PATCH 2/3 nf-next] netfilter:nf_flow_table: Support bridge type flow offload
From: wenxu @ 2019-06-26 10:32 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561545148-11978-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
With nf_conntrack_bridge function. The bridge family can do
conntrack it self. The flow offload function based on the
conntrack. So the flow in the bridge wih conntrack can be
offloaded.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nf_flow_table.h | 30 +++++++++++-
net/netfilter/nf_flow_table_core.c | 53 ++++++++++++++++-----
net/netfilter/nf_flow_table_ip.c | 41 +++++++++++++---
net/netfilter/nft_flow_offload.c | 89 ++++++++++++++++++++++++++++++++---
4 files changed, 185 insertions(+), 28 deletions(-)
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index 968be64..9a0cf27 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -33,8 +33,22 @@ enum flow_offload_tuple_dir {
FLOW_OFFLOAD_DIR_MAX = IP_CT_DIR_MAX
};
+enum flow_offload_tuple_type {
+ FLOW_OFFLOAD_TYPE_INET,
+ FLOW_OFFLOAD_TYPE_BRIDGE,
+};
+
+struct dst_br_port {
+ struct net_device *dev;
+ u16 dst_vlan_tag;
+};
+
struct flow_offload_dst {
- struct dst_entry *dst_cache;
+ enum flow_offload_tuple_type type;
+ union {
+ struct dst_entry *dst_cache;
+ struct dst_br_port dst_port;
+ };
};
struct flow_offload_tuple {
@@ -52,6 +66,7 @@ struct flow_offload_tuple {
};
int iifidx;
+ u16 vlan_tag;
u8 l3proto;
u8 l4proto;
@@ -89,8 +104,19 @@ struct nf_flow_route {
} tuple[FLOW_OFFLOAD_DIR_MAX];
};
+struct nf_flow_forward {
+ struct {
+ struct dst_br_port dst_port;
+ u16 vlan_tag;
+ } tuple[FLOW_OFFLOAD_DIR_MAX];
+};
+
struct nf_flow_data {
- struct nf_flow_route route;
+ enum flow_offload_tuple_type type;
+ union {
+ struct nf_flow_route route;
+ struct nf_flow_forward forward;
+ };
};
struct flow_offload *flow_offload_alloc(struct nf_conn *ct,
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 125ce1c..19ee69c 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -29,16 +29,38 @@ struct flow_offload_entry {
{
struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;
struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple;
- struct dst_entry *other_dst = date->route.tuple[!dir].dst;
- struct dst_entry *dst = data->route.tuple[dir].dst;
+ struct dst_entry *other_dst;
+ struct dst_entry *dst;
+ struct dst_br_port other_dst_port;
+ struct dst_br_port dst_port;
+
+ if (data->type == FLOW_OFFLOAD_TYPE_BRIDGE) {
+ other_dst_port = data->forward.tuple[!dir].dst_port;
+ dst_port = data->forward.tuple[dir].dst_port;
+
+ ft->iifidx = other_dst_port.dev->ifindex;
+ ft->dst.dst_port = dst_port;
+ ft->vlan_tag = data->forward.tuple[dir].vlan_tag;
+ } else {
+ other_dst = data->route.tuple[!dir].dst;
+ dst = data->route.tuple[dir].dst;
+
+ ft->iifidx = other_dst->dev->ifindex;
+ ft->dst.dst_cache = dst;
+ }
+
+ ft->dst.type = data->type;
ft->dir = dir;
switch (ctt->src.l3num) {
case NFPROTO_IPV4:
ft->src_v4 = ctt->src.u3.in;
ft->dst_v4 = ctt->dst.u3.in;
- ft->mtu = ip_dst_mtu_maybe_forward(dst, true);
+ if (data->type == FLOW_OFFLOAD_TYPE_BRIDGE)
+ ft->mtu = dst_port.dev->mtu;
+ else
+ ft->mtu = ip_dst_mtu_maybe_forward(dst, true);
break;
case NFPROTO_IPV6:
ft->src_v6 = ctt->src.u3.in6;
@@ -51,9 +73,6 @@ struct flow_offload_entry {
ft->l4proto = ctt->dst.protonum;
ft->src_port = ctt->src.u.tcp.port;
ft->dst_port = ctt->dst.u.tcp.port;
-
- ft->iifidx = other_dst->dev->ifindex;
- ft->dst_cache = dst;
}
struct flow_offload *
@@ -72,11 +91,13 @@ struct flow_offload *
flow = &entry->flow;
- if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
- goto err_dst_cache_original;
+ if (data->type == FLOW_OFFLOAD_TYPE_INET) {
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
+ goto err_dst_cache_original;
- if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
- goto err_dst_cache_reply;
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
+ goto err_dst_cache_reply;
+ }
entry->ct = ct;
@@ -91,7 +112,8 @@ struct flow_offload *
return flow;
err_dst_cache_reply:
- dst_release(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
+ if (data->type == FLOW_OFFLOAD_TYPE_INET)
+ dst_release(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
err_dst_cache_original:
kfree(entry);
err_ct_refcnt:
@@ -139,8 +161,13 @@ void flow_offload_free(struct flow_offload *flow)
{
struct flow_offload_entry *e;
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+ if (flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.type == FLOW_OFFLOAD_TYPE_INET) {
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
+ } else {
+ dev_put(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_port.dev);
+ dev_put(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_port.dev);
+ }
e = container_of(flow, struct flow_offload_entry, flow);
if (flow->flags & FLOW_OFFLOAD_DYING)
nf_ct_delete(e->ct, 0, 0);
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 0016bb8..9af01ef 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -16,6 +16,8 @@
#include <linux/tcp.h>
#include <linux/udp.h>
+#include "../bridge/br_private.h"
+
static int nf_flow_state_check(struct flow_offload *flow, int proto,
struct sk_buff *skb, unsigned int thoff)
{
@@ -220,6 +222,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
{
struct flow_offload_tuple_rhash *tuplehash;
struct nf_flowtable *flow_table = priv;
+ int family = flow_table->type->family;
struct flow_offload_tuple tuple = {};
enum flow_offload_tuple_dir dir;
struct flow_offload *flow;
@@ -228,6 +231,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
unsigned int thoff;
struct iphdr *iph;
__be32 nexthop;
+ u16 vlan_tag;
if (skb->protocol != htons(ETH_P_IP))
return NF_ACCEPT;
@@ -235,14 +239,25 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
if (nf_flow_tuple_ip(skb, state->in, &tuple) < 0)
return NF_ACCEPT;
+ if (family != NFPROTO_BRIDGE && family != NFPROTO_IPV4)
+ return NF_ACCEPT;
+
+ if (family == NFPROTO_BRIDGE && skb_vlan_tag_present(skb))
+ tuple.vlan_tag = skb_vlan_tag_get_id(skb);
+
tuplehash = flow_offload_lookup(flow_table, &tuple);
if (tuplehash == NULL)
return NF_ACCEPT;
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
- outdev = rt->dst.dev;
+ if (family == NFPROTO_IPV4) {
+ rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
+ outdev = rt->dst.dev;
+ } else {
+ vlan_tag = flow->tuplehash[dir].tuple.dst.dst_port.dst_vlan_tag;
+ outdev = flow->tuplehash[dir].tuple.dst.dst_port.dev;
+ }
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
return NF_ACCEPT;
@@ -258,13 +273,25 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
return NF_DROP;
flow->timeout = (u32)jiffies + NF_FLOW_TIMEOUT;
- iph = ip_hdr(skb);
- ip_decrease_ttl(iph);
skb->dev = outdev;
- nexthop = rt_nexthop(rt, flow->tuplehash[!dir].tuple.src_v4.s_addr);
- skb_dst_set_noref(skb, &rt->dst);
- neigh_xmit(NEIGH_ARP_TABLE, outdev, &nexthop, skb);
+ if (family == NFPROTO_IPV4) {
+ iph = ip_hdr(skb);
+ ip_decrease_ttl(iph);
+
+ nexthop = rt_nexthop(rt, flow->tuplehash[!dir].tuple.src_v4.s_addr);
+ skb_dst_set_noref(skb, &rt->dst);
+ neigh_xmit(NEIGH_ARP_TABLE, outdev, &nexthop, skb);
+ } else {
+ const struct net_bridge_port *p;
+
+ if (vlan_tag && (p = br_port_get_rtnl_rcu(state->in)))
+ __vlan_hwaccel_put_tag(skb, p->br->vlan_proto, vlan_tag);
+ else
+ __vlan_hwaccel_clear_tag(skb);
+
+ br_dev_queue_push_xmit(state->net, state->sk, skb);
+ }
return NF_STOLEN;
}
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index cdb7c46..c88396a 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -14,6 +14,8 @@
#include <linux/netfilter/nf_conntrack_common.h>
#include <net/netfilter/nf_flow_table.h>
+#include "../bridge/br_private.h"
+
struct nft_flow_offload {
struct nft_flowtable *flowtable;
};
@@ -49,6 +51,58 @@ static int nft_flow_route(const struct nft_pktinfo *pkt,
return 0;
}
+static int nft_flow_forward(const struct nft_pktinfo *pkt,
+ const struct nf_conn *ct,
+ struct nf_flow_forward *forward,
+ enum ip_conntrack_dir dir)
+{
+ struct net_bridge_vlan_group *vg;
+ const struct net_bridge_port *p;
+ u16 vid = 0;
+
+ if (skb_vlan_tag_present(pkt->skb))
+ vid = skb_vlan_tag_get_id(pkt->skb);
+
+ forward->tuple[dir].dst_port.dst_vlan_tag = vid;
+ forward->tuple[!dir].vlan_tag = vid;
+ forward->tuple[dir].dst_port.dev = dev_get_by_index(dev_net(nft_out(pkt)),
+ nft_out(pkt)->ifindex);
+ forward->tuple[!dir].dst_port.dev = dev_get_by_index(dev_net(nft_in(pkt)),
+ nft_in(pkt)->ifindex);
+
+ rtnl_lock();
+ p = br_port_get_rtnl_rcu(nft_out(pkt));
+ if (p) {
+ if (!br_opt_get(p->br, BROPT_VLAN_ENABLED))
+ goto out;
+
+ if (!vid) {
+ vg = nbp_vlan_group_rcu(p);
+ vid = br_get_pvid(vg);
+ }
+
+ if (vid) {
+ struct bridge_vlan_info info;
+
+ if (br_vlan_get_info(nft_in(pkt), vid, &info) == 0 &&
+ info.flags & BRIDGE_VLAN_INFO_UNTAGGED)
+ vid = 0;
+ }
+ } else {
+ rtnl_unlock();
+ dev_put(forward->tuple[dir].dst_port.dev);
+ dev_put(forward->tuple[!dir].dst_port.dev);
+ return -ENOENT;
+ }
+
+out:
+ rtnl_unlock();
+ forward->tuple[!dir].dst_port.dst_vlan_tag = vid;
+ forward->tuple[dir].vlan_tag = vid;
+
+ return 0;
+}
+
static bool nft_flow_offload_skip(struct sk_buff *skb, int family)
{
if (skb_sec_path(skb))
@@ -61,6 +115,15 @@ static bool nft_flow_offload_skip(struct sk_buff *skb, int family)
if (unlikely(opt->optlen))
return true;
+ } else if (family == NFPROTO_BRIDGE) {
+ const struct iphdr *iph;
+
+ if (skb->protocol != htons(ETH_P_IP))
+ return true;
+
+ iph = ip_hdr(skb);
+ if (iph->ihl > 5)
+ return true;
}
return false;
@@ -76,11 +139,12 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nf_flow_data data;
struct flow_offload *flow;
enum ip_conntrack_dir dir;
+ int family = nft_pf(pkt);
bool is_tcp = false;
struct nf_conn *ct;
int ret;
- if (nft_flow_offload_skip(pkt->skb, nft_pf(pkt)))
+ if (nft_flow_offload_skip(pkt->skb, family))
goto out;
ct = nf_ct_get(pkt->skb, &ctinfo);
@@ -108,8 +172,15 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
goto out;
dir = CTINFO2DIR(ctinfo);
- if (nft_flow_route(pkt, ct, &data.route, dir) < 0)
- goto err_flow_route;
+ if (family == NFPROTO_BRIDGE) {
+ data.type = FLOW_OFFLOAD_TYPE_BRIDGE;
+ if (nft_flow_forward(pkt, ct, &data.forward, dir) < 0)
+ goto err_flow_data;
+ } else {
+ data.type = FLOW_OFFLOAD_TYPE_INET;
+ if (nft_flow_route(pkt, ct, &data.route, dir) < 0)
+ goto err_flow_data;
+ }
flow = flow_offload_alloc(ct, &data);
if (!flow)
@@ -124,14 +195,20 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
if (ret < 0)
goto err_flow_add;
- dst_release(data.route.tuple[!dir].dst);
+ if (family != NFPROTO_BRIDGE)
+ dst_release(data.route.tuple[!dir].dst);
return;
err_flow_add:
flow_offload_free(flow);
err_flow_alloc:
- dst_release(data.route.tuple[!dir].dst);
-err_flow_route:
+ if (family == NFPROTO_BRIDGE) {
+ dev_put(data.forward.tuple[dir].dst_port.dev);
+ dev_put(data.forward.tuple[!dir].dst_port.dev);
+ } else {
+ dst_release(data.route.tuple[!dir].dst);
+ }
+err_flow_data:
clear_bit(IPS_OFFLOAD_BIT, &ct->status);
out:
regs->verdict.code = NFT_BREAK;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/3 nf-next] netfilter:nf_flow_table: Refactor flow_offload_tuple to support more offload method
From: wenxu @ 2019-06-26 10:32 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
From: wenxu <wenxu@ucloud.cn>
Add struct flow_offload_dst to support more offload method to replace
dst_cache which only work for route offload.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/net/netfilter/nf_flow_table.h | 12 ++++++++++--
net/netfilter/nf_flow_table_core.c | 22 +++++++++++-----------
net/netfilter/nf_flow_table_ip.c | 4 ++--
net/netfilter/nft_flow_offload.c | 10 +++++-----
4 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index d8c1879..968be64 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -33,6 +33,10 @@ enum flow_offload_tuple_dir {
FLOW_OFFLOAD_DIR_MAX = IP_CT_DIR_MAX
};
+struct flow_offload_dst {
+ struct dst_entry *dst_cache;
+};
+
struct flow_offload_tuple {
union {
struct in_addr src_v4;
@@ -55,7 +59,7 @@ struct flow_offload_tuple {
u16 mtu;
- struct dst_entry *dst_cache;
+ struct flow_offload_dst dst;
};
struct flow_offload_tuple_rhash {
@@ -85,8 +89,12 @@ struct nf_flow_route {
} tuple[FLOW_OFFLOAD_DIR_MAX];
};
+struct nf_flow_data {
+ struct nf_flow_route route;
+};
+
struct flow_offload *flow_offload_alloc(struct nf_conn *ct,
- struct nf_flow_route *route);
+ struct nf_flow_data *data);
void flow_offload_free(struct flow_offload *flow);
int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow);
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index e3d7972..125ce1c 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -24,13 +24,13 @@ struct flow_offload_entry {
static void
flow_offload_fill_dir(struct flow_offload *flow, struct nf_conn *ct,
- struct nf_flow_route *route,
+ struct nf_flow_data *data,
enum flow_offload_tuple_dir dir)
{
struct flow_offload_tuple *ft = &flow->tuplehash[dir].tuple;
struct nf_conntrack_tuple *ctt = &ct->tuplehash[dir].tuple;
- struct dst_entry *other_dst = route->tuple[!dir].dst;
- struct dst_entry *dst = route->tuple[dir].dst;
+ struct dst_entry *other_dst = date->route.tuple[!dir].dst;
+ struct dst_entry *dst = data->route.tuple[dir].dst;
ft->dir = dir;
@@ -57,7 +57,7 @@ struct flow_offload_entry {
}
struct flow_offload *
-flow_offload_alloc(struct nf_conn *ct, struct nf_flow_route *route)
+flow_offload_alloc(struct nf_conn *ct, struct nf_flow_data *data)
{
struct flow_offload_entry *entry;
struct flow_offload *flow;
@@ -72,16 +72,16 @@ struct flow_offload *
flow = &entry->flow;
- if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst))
goto err_dst_cache_original;
- if (!dst_hold_safe(route->tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
+ if (!dst_hold_safe(data->route.tuple[FLOW_OFFLOAD_DIR_REPLY].dst))
goto err_dst_cache_reply;
entry->ct = ct;
- flow_offload_fill_dir(flow, ct, route, FLOW_OFFLOAD_DIR_ORIGINAL);
- flow_offload_fill_dir(flow, ct, route, FLOW_OFFLOAD_DIR_REPLY);
+ flow_offload_fill_dir(flow, ct, data, FLOW_OFFLOAD_DIR_ORIGINAL);
+ flow_offload_fill_dir(flow, ct, data, FLOW_OFFLOAD_DIR_REPLY);
if (ct->status & IPS_SRC_NAT)
flow->flags |= FLOW_OFFLOAD_SNAT;
@@ -91,7 +91,7 @@ struct flow_offload *
return flow;
err_dst_cache_reply:
- dst_release(route->tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
+ dst_release(data->route.tuple[FLOW_OFFLOAD_DIR_ORIGINAL].dst);
err_dst_cache_original:
kfree(entry);
err_ct_refcnt:
@@ -139,8 +139,8 @@ void flow_offload_free(struct flow_offload *flow)
{
struct flow_offload_entry *e;
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst_cache);
- dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst.dst_cache);
+ dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst.dst_cache);
e = container_of(flow, struct flow_offload_entry, flow);
if (flow->flags & FLOW_OFFLOAD_DYING)
nf_ct_delete(e->ct, 0, 0);
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index 2413174..0016bb8 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -241,7 +241,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rtable *)flow->tuplehash[dir].tuple.dst_cache;
+ rt = (struct rtable *)flow->tuplehash[dir].tuple.dst.dst_cache;
outdev = rt->dst.dev;
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
@@ -457,7 +457,7 @@ static int nf_flow_tuple_ipv6(struct sk_buff *skb, const struct net_device *dev,
dir = tuplehash->tuple.dir;
flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
- rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst_cache;
+ rt = (struct rt6_info *)flow->tuplehash[dir].tuple.dst.dst_cache;
outdev = rt->dst.dev;
if (unlikely(nf_flow_exceeds_mtu(skb, flow->tuplehash[dir].tuple.mtu)))
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index aa5f571..cdb7c46 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -73,7 +73,7 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
struct nft_flow_offload *priv = nft_expr_priv(expr);
struct nf_flowtable *flowtable = &priv->flowtable->data;
enum ip_conntrack_info ctinfo;
- struct nf_flow_route route;
+ struct nf_flow_data data;
struct flow_offload *flow;
enum ip_conntrack_dir dir;
bool is_tcp = false;
@@ -108,10 +108,10 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
goto out;
dir = CTINFO2DIR(ctinfo);
- if (nft_flow_route(pkt, ct, &route, dir) < 0)
+ if (nft_flow_route(pkt, ct, &data.route, dir) < 0)
goto err_flow_route;
- flow = flow_offload_alloc(ct, &route);
+ flow = flow_offload_alloc(ct, &data);
if (!flow)
goto err_flow_alloc;
@@ -124,13 +124,13 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
if (ret < 0)
goto err_flow_add;
- dst_release(route.tuple[!dir].dst);
+ dst_release(data.route.tuple[!dir].dst);
return;
err_flow_add:
flow_offload_free(flow);
err_flow_alloc:
- dst_release(route.tuple[!dir].dst);
+ dst_release(data.route.tuple[!dir].dst);
err_flow_route:
clear_bit(IPS_OFFLOAD_BIT, &ct->status);
out:
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/3 nf-next] netfilter: Flow table support for the bridge family
From: wenxu @ 2019-06-26 10:32 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1561545148-11978-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
This patch adds the bridge flow table type, that implements the datapath
flow table to forward IPv4 traffic through bridge.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/bridge/netfilter/Kconfig | 8 +++++
net/bridge/netfilter/Makefile | 1 +
net/bridge/netfilter/nf_flow_table_bridge.c | 46 +++++++++++++++++++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 net/bridge/netfilter/nf_flow_table_bridge.c
diff --git a/net/bridge/netfilter/Kconfig b/net/bridge/netfilter/Kconfig
index f4fb0b9..cba5f71 100644
--- a/net/bridge/netfilter/Kconfig
+++ b/net/bridge/netfilter/Kconfig
@@ -33,6 +33,14 @@ config NF_CONNTRACK_BRIDGE
To compile it as a module, choose M here. If unsure, say N.
+config NF_FLOW_TABLE_BRIDGE
+ tristate "Netfilter flow table bridge module"
+ depends on NF_FLOW_TABLE && NF_CONNTRACK_BRIDGE
+ help
+ This option adds the flow table bridge support.
+
+ To compile it as a module, choose M here.
+
endif # NF_TABLES_BRIDGE
menuconfig BRIDGE_NF_EBTABLES
diff --git a/net/bridge/netfilter/Makefile b/net/bridge/netfilter/Makefile
index 9d77673..deb81e6 100644
--- a/net/bridge/netfilter/Makefile
+++ b/net/bridge/netfilter/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_NFT_BRIDGE_REJECT) += nft_reject_bridge.o
# connection tracking
obj-$(CONFIG_NF_CONNTRACK_BRIDGE) += nf_conntrack_bridge.o
+obj-$(CONFIG_NF_FLOW_TABLE_BRIDGE) += nf_flow_table_bridge.o
# packet logging
obj-$(CONFIG_NF_LOG_BRIDGE) += nf_log_bridge.o
diff --git a/net/bridge/netfilter/nf_flow_table_bridge.c b/net/bridge/netfilter/nf_flow_table_bridge.c
new file mode 100644
index 0000000..ad3220c
--- /dev/null
+++ b/net/bridge/netfilter/nf_flow_table_bridge.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/netfilter.h>
+#include <net/netfilter/nf_flow_table.h>
+#include <net/netfilter/nf_tables.h>
+
+static unsigned int
+nf_flow_offload_bridge_hook(void *priv, struct sk_buff *skb,
+ const struct nf_hook_state *state)
+{
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ return nf_flow_offload_ip_hook(priv, skb, state);
+ }
+
+ return NF_ACCEPT;
+}
+
+static struct nf_flowtable_type flowtable_bridge = {
+ .family = NFPROTO_BRIDGE,
+ .init = nf_flow_table_init,
+ .free = nf_flow_table_free,
+ .hook = nf_flow_offload_bridge_hook,
+ .owner = THIS_MODULE,
+};
+
+static int __init nf_flow_bridge_module_init(void)
+{
+ nft_register_flowtable_type(&flowtable_bridge);
+
+ return 0;
+}
+
+static void __exit nf_flow_bridge_module_exit(void)
+{
+ nft_unregister_flowtable_type(&flowtable_bridge);
+}
+
+module_init(nf_flow_bridge_module_init);
+module_exit(nf_flow_bridge_module_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("wenxu <wenxu@ucloud.cn>");
+MODULE_ALIAS_NF_FLOWTABLE(AF_BRIDGE);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] ipv6: fix suspicious RCU usage in rt6_dump_route()
From: Stefano Brivio @ 2019-06-26 10:34 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, David Ahern
In-Reply-To: <20190626100528.218097-1-edumazet@google.com>
On Wed, 26 Jun 2019 03:05:28 -0700
Eric Dumazet <edumazet@google.com> wrote:
> syzbot reminded us that rt6_nh_dump_exceptions() needs to be called
> with rcu_read_lock()
>
> net/ipv6/route.c:1593 suspicious rcu_dereference_check() usage!
Thanks for fixing this too.
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply
* [PATCH v2 bpf-next] libbpf: fix max() type mismatch for 32bit
From: Ivan Khoronzhuk @ 2019-06-26 10:38 UTC (permalink / raw)
To: ast, netdev; +Cc: daniel, bpf, linux-kernel, Ivan Khoronzhuk
It fixes build error for 32bit caused by type mismatch
size_t/unsigned long.
Fixes: bf82927125dd ("libbpf: refactor map initialization")
Acked-by: Song Liu <songliubraving@fb.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 68f45a96769f..5186b7710430 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -778,7 +778,7 @@ static struct bpf_map *bpf_object__add_map(struct bpf_object *obj)
if (obj->nr_maps < obj->maps_cap)
return &obj->maps[obj->nr_maps++];
- new_cap = max(4ul, obj->maps_cap * 3 / 2);
+ new_cap = max((size_t)4, obj->maps_cap * 3 / 2);
new_maps = realloc(obj->maps, new_cap * sizeof(*obj->maps));
if (!new_maps) {
pr_warning("alloc maps for object failed\n");
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Jesper Dangaard Brouer @ 2019-06-26 10:42 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: davem, grygorii.strashko, hawk, saeedm, leon, ast, linux-kernel,
linux-omap, xdp-newbies, ilias.apalodimas, netdev, daniel,
jakub.kicinski, john.fastabend, brouer
In-Reply-To: <20190625175948.24771-2-ivan.khoronzhuk@linaro.org>
On Tue, 25 Jun 2019 20:59:45 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> Add user counter allowing to delete pool only when no users.
> It doesn't prevent pool from flush, only prevents freeing the
> pool instance. Helps when no need to delete the pool and now
> it's user responsibility to free it by calling page_pool_free()
> while destroying procedure. It also makes to use page_pool_free()
> explicitly, not fully hidden in xdp unreg, which looks more
> correct after page pool "create" routine.
No, this is wrong.
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 8 +++++---
> include/net/page_pool.h | 7 +++++++
> net/core/page_pool.c | 7 +++++++
> net/core/xdp.c | 3 +++
> 4 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 5e40db8f92e6..cb028de64a1d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -545,10 +545,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> }
> err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
> MEM_TYPE_PAGE_POOL, rq->page_pool);
> - if (err) {
> - page_pool_free(rq->page_pool);
> + if (err)
> goto err_free;
> - }
>
> for (i = 0; i < wq_sz; i++) {
> if (rq->wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ) {
> @@ -613,6 +611,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> if (rq->xdp_prog)
> bpf_prog_put(rq->xdp_prog);
> xdp_rxq_info_unreg(&rq->xdp_rxq);
> + if (rq->page_pool)
> + page_pool_free(rq->page_pool);
> mlx5_wq_destroy(&rq->wq_ctrl);
>
> return err;
> @@ -643,6 +643,8 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
> }
>
> xdp_rxq_info_unreg(&rq->xdp_rxq);
> + if (rq->page_pool)
> + page_pool_free(rq->page_pool);
No, this is wrong. The hole point with the merged page_pool fixes
patchset was that page_pool_free() needs to be delayed until no-more
in-flight packets exist.
> mlx5_wq_destroy(&rq->wq_ctrl);
> }
>
> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
> index f07c518ef8a5..1ec838e9927e 100644
> --- a/include/net/page_pool.h
> +++ b/include/net/page_pool.h
> @@ -101,6 +101,7 @@ struct page_pool {
> struct ptr_ring ring;
>
> atomic_t pages_state_release_cnt;
> + atomic_t user_cnt;
> };
>
> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
> @@ -183,6 +184,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> return page->dma_addr;
> }
>
> +/* used to prevent pool from deallocation */
> +static inline void page_pool_get(struct page_pool *pool)
> +{
> + atomic_inc(&pool->user_cnt);
> +}
> +
> static inline bool is_page_pool_compiled_in(void)
> {
> #ifdef CONFIG_PAGE_POOL
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index b366f59885c1..169b0e3c870e 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -48,6 +48,7 @@ static int page_pool_init(struct page_pool *pool,
> return -ENOMEM;
>
> atomic_set(&pool->pages_state_release_cnt, 0);
> + atomic_set(&pool->user_cnt, 0);
>
> if (pool->p.flags & PP_FLAG_DMA_MAP)
> get_device(pool->p.dev);
> @@ -70,6 +71,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
> kfree(pool);
> return ERR_PTR(err);
> }
> +
> + page_pool_get(pool);
> return pool;
> }
> EXPORT_SYMBOL(page_pool_create);
> @@ -356,6 +359,10 @@ static void __warn_in_flight(struct page_pool *pool)
>
> void __page_pool_free(struct page_pool *pool)
> {
> + /* free only if no users */
> + if (!atomic_dec_and_test(&pool->user_cnt))
> + return;
> +
> WARN(pool->alloc.count, "API usage violation");
> WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 829377cc83db..04bdcd784d2e 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -372,6 +372,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>
> mutex_unlock(&mem_id_lock);
>
> + if (type == MEM_TYPE_PAGE_POOL)
> + page_pool_get(xdp_alloc->page_pool);
> +
> trace_mem_connect(xdp_alloc, xdp_rxq);
> return 0;
> err:
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH 1/2] net: stmmac: Fix possible deadlock when disabling EEE support
From: Thierry Reding @ 2019-06-26 10:45 UTC (permalink / raw)
To: Jon Hunter
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, netdev,
linux-kernel, linux-tegra
In-Reply-To: <20190626102322.18821-1-jonathanh@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 721 bytes --]
On Wed, Jun 26, 2019 at 11:23:21AM +0100, Jon Hunter wrote:
> When stmmac_eee_init() is called to disable EEE support, then the timer
> for EEE support is stopped and we return from the function. Prior to
> stopping the timer, a mutex was acquired but in this case it is never
> released and so could cause a deadlock. Fix this by releasing the mutex
> prior to returning from stmmax_eee_init() when stopping the EEE timer.
>
> Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
> 1 file changed, 1 insertion(+)
Tested-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] net: stmmac: Fix crash observed if PHY does not support EEE
From: Thierry Reding @ 2019-06-26 10:45 UTC (permalink / raw)
To: Jon Hunter
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, netdev,
linux-kernel, linux-tegra
In-Reply-To: <20190626102322.18821-2-jonathanh@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]
On Wed, Jun 26, 2019 at 11:23:22AM +0100, Jon Hunter wrote:
> If the PHY does not support EEE mode, then a crash is observed when the
> ethernet interface is enabled. The crash occurs, because if the PHY does
> not support EEE, then although the EEE timer is never configured, it is
> still marked as enabled and so the stmmac ethernet driver is still
> trying to update the timer by calling mod_timer(). This triggers a BUG()
> in the mod_timer() because we are trying to update a timer when there is
> no callback function set because timer_setup() was never called for this
> timer.
>
> The problem is caused because we return true from the function
> stmmac_eee_init(), marking the EEE timer as enabled, even when we have
> not configured the EEE timer. Fix this by ensuring that we return false
> if the PHY does not support EEE and hence, 'eee_active' is not set.
>
> Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
Thanks for hunting this down!
Tested-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Ivan Khoronzhuk @ 2019-06-26 10:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: davem, grygorii.strashko, hawk, saeedm, leon, ast, linux-kernel,
linux-omap, xdp-newbies, ilias.apalodimas, netdev, daniel,
jakub.kicinski, john.fastabend
In-Reply-To: <20190626124216.494eee86@carbon>
On Wed, Jun 26, 2019 at 12:42:16PM +0200, Jesper Dangaard Brouer wrote:
>On Tue, 25 Jun 2019 20:59:45 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> Add user counter allowing to delete pool only when no users.
>> It doesn't prevent pool from flush, only prevents freeing the
>> pool instance. Helps when no need to delete the pool and now
>> it's user responsibility to free it by calling page_pool_free()
>> while destroying procedure. It also makes to use page_pool_free()
>> explicitly, not fully hidden in xdp unreg, which looks more
>> correct after page pool "create" routine.
>
>No, this is wrong.
below.
>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 8 +++++---
>> include/net/page_pool.h | 7 +++++++
>> net/core/page_pool.c | 7 +++++++
>> net/core/xdp.c | 3 +++
>> 4 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index 5e40db8f92e6..cb028de64a1d 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -545,10 +545,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
>> }
>> err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
>> MEM_TYPE_PAGE_POOL, rq->page_pool);
>> - if (err) {
>> - page_pool_free(rq->page_pool);
>> + if (err)
>> goto err_free;
>> - }
>>
>> for (i = 0; i < wq_sz; i++) {
>> if (rq->wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ) {
>> @@ -613,6 +611,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
>> if (rq->xdp_prog)
>> bpf_prog_put(rq->xdp_prog);
>> xdp_rxq_info_unreg(&rq->xdp_rxq);
>> + if (rq->page_pool)
>> + page_pool_free(rq->page_pool);
>> mlx5_wq_destroy(&rq->wq_ctrl);
>>
>> return err;
>> @@ -643,6 +643,8 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
>> }
>>
>> xdp_rxq_info_unreg(&rq->xdp_rxq);
>> + if (rq->page_pool)
>> + page_pool_free(rq->page_pool);
>
>No, this is wrong. The hole point with the merged page_pool fixes
>patchset was that page_pool_free() needs to be delayed until no-more
>in-flight packets exist.
Probably it's not so obvious, but it's still delayed and deleted only
after no-more in-flight packets exist. Here question is only who is able
to do this first based on refcnt.
>
>
>> mlx5_wq_destroy(&rq->wq_ctrl);
>> }
>>
>> diff --git a/include/net/page_pool.h b/include/net/page_pool.h
>> index f07c518ef8a5..1ec838e9927e 100644
>> --- a/include/net/page_pool.h
>> +++ b/include/net/page_pool.h
>> @@ -101,6 +101,7 @@ struct page_pool {
>> struct ptr_ring ring;
>>
>> atomic_t pages_state_release_cnt;
>> + atomic_t user_cnt;
>> };
>>
>> struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
>> @@ -183,6 +184,12 @@ static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
>> return page->dma_addr;
>> }
>>
>> +/* used to prevent pool from deallocation */
>> +static inline void page_pool_get(struct page_pool *pool)
>> +{
>> + atomic_inc(&pool->user_cnt);
>> +}
>> +
>> static inline bool is_page_pool_compiled_in(void)
>> {
>> #ifdef CONFIG_PAGE_POOL
>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
>> index b366f59885c1..169b0e3c870e 100644
>> --- a/net/core/page_pool.c
>> +++ b/net/core/page_pool.c
>> @@ -48,6 +48,7 @@ static int page_pool_init(struct page_pool *pool,
>> return -ENOMEM;
>>
>> atomic_set(&pool->pages_state_release_cnt, 0);
>> + atomic_set(&pool->user_cnt, 0);
>>
>> if (pool->p.flags & PP_FLAG_DMA_MAP)
>> get_device(pool->p.dev);
>> @@ -70,6 +71,8 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
>> kfree(pool);
>> return ERR_PTR(err);
>> }
>> +
>> + page_pool_get(pool);
>> return pool;
>> }
>> EXPORT_SYMBOL(page_pool_create);
>> @@ -356,6 +359,10 @@ static void __warn_in_flight(struct page_pool *pool)
>>
>> void __page_pool_free(struct page_pool *pool)
>> {
>> + /* free only if no users */
>> + if (!atomic_dec_and_test(&pool->user_cnt))
>> + return;
>> +
>> WARN(pool->alloc.count, "API usage violation");
>> WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
>>
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 829377cc83db..04bdcd784d2e 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -372,6 +372,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>>
>> mutex_unlock(&mem_id_lock);
>>
>> + if (type == MEM_TYPE_PAGE_POOL)
>> + page_pool_get(xdp_alloc->page_pool);
>> +
>> trace_mem_connect(xdp_alloc, xdp_rxq);
>> return 0;
>> err:
>
>
>
>--
>Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH v4 net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Neil Horman @ 2019-06-26 10:54 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <CAF=yD-+fCNGQyoRNAZngof3=_gGbHn9aSCQA_hNvFSsSZtZQxA@mail.gmail.com>
On Tue, Jun 25, 2019 at 06:30:08PM -0400, Willem de Bruijn wrote:
> > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > index a29d66da7394..a7ca6a003ebe 100644
> > --- a/net/packet/af_packet.c
> > +++ b/net/packet/af_packet.c
> > @@ -2401,6 +2401,9 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
> >
> > ts = __packet_set_timestamp(po, ph, skb);
> > __packet_set_status(po, ph, TP_STATUS_AVAILABLE | ts);
> > +
> > + if (!packet_read_pending(&po->tx_ring))
> > + complete(&po->skb_completion);
> > }
> >
> > sock_wfree(skb);
> > @@ -2585,7 +2588,7 @@ static int tpacket_parse_header(struct packet_sock *po, void *frame,
> >
> > static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> > {
> > - struct sk_buff *skb;
> > + struct sk_buff *skb = NULL;
> > struct net_device *dev;
> > struct virtio_net_hdr *vnet_hdr = NULL;
> > struct sockcm_cookie sockc;
> > @@ -2600,6 +2603,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> > int len_sum = 0;
> > int status = TP_STATUS_AVAILABLE;
> > int hlen, tlen, copylen = 0;
> > + long timeo = 0;
> >
> > mutex_lock(&po->pg_vec_lock);
> >
> > @@ -2646,12 +2650,21 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> > if ((size_max > dev->mtu + reserve + VLAN_HLEN) && !po->has_vnet_hdr)
> > size_max = dev->mtu + reserve + VLAN_HLEN;
> >
> > + reinit_completion(&po->skb_completion);
> > +
> > do {
> > ph = packet_current_frame(po, &po->tx_ring,
> > TP_STATUS_SEND_REQUEST);
> > if (unlikely(ph == NULL)) {
> > - if (need_wait && need_resched())
> > - schedule();
> > + if (need_wait && skb) {
> > + timeo = sock_sndtimeo(&po->sk, msg->msg_flags & MSG_DONTWAIT);
> > + timeo = wait_for_completion_interruptible_timeout(&po->skb_completion, timeo);
>
> This looks really nice.
>
> But isn't it still susceptible to the race where tpacket_destruct_skb
> is called in between po->xmit and this
> wait_for_completion_interruptible_timeout?
>
Thats not an issue, since the complete is only gated on packet_read_pending
reaching 0 in tpacket_destuct_skb. Previously it was gated on my
wait_on_complete flag being non-zero, so we had to set that prior to calling
po->xmit, or the complete call might never get made, resulting in a hang. Now,
we will always call complete, and the completion api allows for arbitrary
ordering of complete/wait_for_complete (since its internal done variable gets
incremented), making a call to wait_for_complete effectively a fall through if
complete gets called first.
There is an odd path here though. If an application calls sendmsg on a packet
socket here with MSG_DONTWAIT set, then need_wait will be zero, and we will
eventually exit this loop without ever having called wait_for_complete, but
tpacket_destruct_skb will still have called complete when all the frames
complete transmission. In and of itself, thats fine, but it leave the
completion structure in a state where its done variable will have been
incremented at least once (specifically it will be set to N, where N is the
number of frames transmitted during the call where MSG_DONTWAIT is set). If the
application then calls sendmsg on this socket with MSG_DONTWAIT clear, we will
call wait_for_complete, but immediately return from it (due to the previously
made calls to complete). I've corrected this however, but adding that call to
reinit_completion prior to the loop entry, so that we are always guaranteed to
have the completion variable set properly to wait for only the frames being sent
in this particular instance of the sendmsg call.
> The test for skb is shorthand for packet_read_pending != 0, right?
>
Sort of. gating on skb guarantees for us that we have sent at least one frame
in this call to tpacket_snd. If we didn't do that, then it would be possible
for an application to call sendmsg without setting any frames in the buffer to
TP_STATUS_SEND_REQUEST, which would cause us to wait for a completion without
having sent any frames, meaning we would block waiting for an event
(tpacket_destruct_skb), that will never happen. The check for skb ensures that
tpacket_snd_skb will get called, and that we will get a wakeup from a call to
wait_for_complete. It does suggest that packet_read_pending != 0, but thats not
guaranteed, because tpacket_destruct_skb may already have been called (see the
above explination regarding ordering of complete/wait_for_complete).
Neil
^ permalink raw reply
* Question about nf_conntrack_proto for IPsec
From: Naruto Nguyen @ 2019-06-26 11:06 UTC (permalink / raw)
To: netfilter-devel, netdev, netfilter
Hi everyone,
In linux/latest/source/net/netfilter/ folder, I only see we have
nf_conntrack_proto_tcp.c, nf_conntrack_proto_udp.c and some other
conntrack implementations for other protocols but I do not see
nf_conntrack_proto for IPsec, so does it mean connection tracking
cannot track ESP or AH protocol as a connection. I mean when I use
"conntrack -L" command, I will not see ESP or AH connection is saved
in conntrack list. Could you please help me to understand if conntrack
supports that and any reasons if it does not support?
Thanks a lot,
Brs,
Naruto
^ permalink raw reply
* Re: Question about nf_conntrack_proto for IPsec
From: Florian Westphal @ 2019-06-26 11:13 UTC (permalink / raw)
To: Naruto Nguyen; +Cc: netfilter-devel, netdev, netfilter
In-Reply-To: <CANpxKHHXzrEpJPSj3x83+WE23G1W0KPz9XbG=fCVzS21+-BpfQ@mail.gmail.com>
Naruto Nguyen <narutonguyen2018@gmail.com> wrote:
> In linux/latest/source/net/netfilter/ folder, I only see we have
> nf_conntrack_proto_tcp.c, nf_conntrack_proto_udp.c and some other
> conntrack implementations for other protocols but I do not see
> nf_conntrack_proto for IPsec, so does it mean connection tracking
> cannot track ESP or AH protocol as a connection. I mean when I use
> "conntrack -L" command, I will not see ESP or AH connection is saved
> in conntrack list. Could you please help me to understand if conntrack
> supports that and any reasons if it does not support?
ESP/AH etc. use the generic tracker, i.e. only one ESP connection
is tracked between each endpoint.
^ permalink raw reply
* [PATCH net-next 0/3] Better PHYLINK compliance for SJA1105 DSA
From: Vladimir Oltean @ 2019-06-26 11:20 UTC (permalink / raw)
To: rmk+kernel, f.fainelli, vivien.didelot, andrew, davem
Cc: netdev, Vladimir Oltean
After discussing with Russell King, it appears this driver is making a
few confusions and not performing some checks for consistent operation.
Vladimir Oltean (3):
net: dsa: sja1105: Don't check state->link in phylink_mac_config
net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK
reports
net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK
drivers/net/dsa/sja1105/sja1105_main.c | 59 +++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 1/3] net: dsa: sja1105: Don't check state->link in phylink_mac_config
From: Vladimir Oltean @ 2019-06-26 11:20 UTC (permalink / raw)
To: rmk+kernel, f.fainelli, vivien.didelot, andrew, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190626112014.7625-1-olteanv@gmail.com>
It has been pointed out that PHYLINK can call mac_config only to update
the phy_interface_type and without knowing what the AN results are.
Experimentally, when this was observed to happen, state->link was also
unset, and therefore was used as a proxy to ignore this call. However it
is also suggested that state->link is undefined for this callback and
should not be relied upon.
So let the previously-dead codepath for SPEED_UNKNOWN be called, and
update the comment to make sure the MAC's behavior is sane.
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index caebf76eaa3e..da1736093b06 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -715,7 +715,13 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
switch (speed_mbps) {
case SPEED_UNKNOWN:
- /* No speed update requested */
+ /* PHYLINK called sja1105_mac_config() to inform us about
+ * the state->interface, but AN has not completed and the
+ * speed is not yet valid. UM10944.pdf says that setting
+ * SJA1105_SPEED_AUTO at runtime disables the port, so that is
+ * ok for power consumption in case AN will never complete -
+ * otherwise PHYLINK should come back with a new update.
+ */
speed = SJA1105_SPEED_AUTO;
break;
case SPEED_10:
@@ -766,9 +772,6 @@ static void sja1105_mac_config(struct dsa_switch *ds, int port,
{
struct sja1105_private *priv = ds->priv;
- if (!state->link)
- return;
-
sja1105_adjust_port_config(priv, port, state->speed);
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/3] net: dsa: sja1105: Check for PHY mode mismatches with what PHYLINK reports
From: Vladimir Oltean @ 2019-06-26 11:20 UTC (permalink / raw)
To: rmk+kernel, f.fainelli, vivien.didelot, andrew, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190626112014.7625-1-olteanv@gmail.com>
PHYLINK being designed with PHYs in mind that can change MII protocol,
for correct operation it is necessary to ensure that the PHY interface
mode stays the same (otherwise clear the supported bit mask, as
required).
Because this is just a hypothetical situation for now, we don't bother
to check whether we could actually support the new PHY interface mode.
Actually we could modify the xMII table, reset the switch and send an
updated static configuration, but adding that would just be dead code.
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 47 ++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index da1736093b06..ad4f604590c0 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -766,12 +766,46 @@ static int sja1105_adjust_port_config(struct sja1105_private *priv, int port,
return sja1105_clocking_setup_port(priv, port);
}
+/* The SJA1105 MAC programming model is through the static config (the xMII
+ * Mode table cannot be dynamically reconfigured), and we have to program
+ * that early (earlier than PHYLINK calls us, anyway).
+ * So just error out in case the connected PHY attempts to change the initial
+ * system interface MII protocol from what is defined in the DT, at least for
+ * now.
+ */
+static bool sja1105_phy_mode_mismatch(struct sja1105_private *priv, int port,
+ phy_interface_t interface)
+{
+ struct sja1105_xmii_params_entry *mii;
+ sja1105_phy_interface_t phy_mode;
+
+ mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
+ phy_mode = mii->xmii_mode[port];
+
+ switch (interface) {
+ case PHY_INTERFACE_MODE_MII:
+ return (phy_mode != XMII_MODE_MII);
+ case PHY_INTERFACE_MODE_RMII:
+ return (phy_mode != XMII_MODE_RMII);
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ return (phy_mode != XMII_MODE_RGMII);
+ default:
+ return true;
+ }
+}
+
static void sja1105_mac_config(struct dsa_switch *ds, int port,
unsigned int link_an_mode,
const struct phylink_link_state *state)
{
struct sja1105_private *priv = ds->priv;
+ if (sja1105_phy_mode_mismatch(priv, port, state->interface))
+ return;
+
sja1105_adjust_port_config(priv, port, state->speed);
}
@@ -804,6 +838,19 @@ static void sja1105_phylink_validate(struct dsa_switch *ds, int port,
mii = priv->static_config.tables[BLK_IDX_XMII_PARAMS].entries;
+ /* include/linux/phylink.h says:
+ * When @state->interface is %PHY_INTERFACE_MODE_NA, phylink
+ * expects the MAC driver to return all supported link modes.
+ */
+ if (state->interface != PHY_INTERFACE_MODE_NA &&
+ sja1105_phy_mode_mismatch(priv, port, state->interface)) {
+ dev_warn(ds->dev, "PHY mode mismatch on port %d: "
+ "PHYLINK tried to change to %s\n",
+ port, phy_modes(state->interface));
+ bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+ return;
+ }
+
/* The MAC does not support pause frames, and also doesn't
* support half-duplex traffic modes.
*/
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 3/3] net: dsa: sja1105: Mark in-band AN modes not supported for PHYLINK
From: Vladimir Oltean @ 2019-06-26 11:20 UTC (permalink / raw)
To: rmk+kernel, f.fainelli, vivien.didelot, andrew, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190626112014.7625-1-olteanv@gmail.com>
We need a better way to signal this, perhaps in phylink_validate, but
for now just print this error message as guidance for other people
looking at this driver's code while trying to rework PHYLINK.
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index ad4f604590c0..d82afb835fb7 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -806,6 +806,11 @@ static void sja1105_mac_config(struct dsa_switch *ds, int port,
if (sja1105_phy_mode_mismatch(priv, port, state->interface))
return;
+ if (link_an_mode == MLO_AN_INBAND) {
+ dev_err(ds->dev, "In-band AN not supported!\n");
+ return;
+ }
+
sja1105_adjust_port_config(priv, port, state->speed);
}
--
2.17.1
^ permalink raw reply related
* net: never suspend the ethernet PHY on certain boards?
From: Alexander Dahl @ 2019-06-26 11:23 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Thomas Pfahl
Hei hei,
tl;dr: is there a way to prevent an ethernet PHY to ever power down, preferred
with some dt configuration, not with a hack e.g. patching out suspend
functions?
With the bugfix 0da70f808029476001109b6cb076737bc04cea2e ("net: macb: do not
disable MDIO bus at open/close time", came with kernel v4.19, was backported
to v4.18.7) a problem arises for us, which was masked before for ages, with a
special combination of SoC, ethernet PHY and other chips on the same board,
and the linux drivers for that.
The boards use either a at91sam9g20 or a sama5d27 SoC, both using cadence/macb
as ethernet driver. Both boards have a smsc LAN8720A ethernet phy attached.
The RMII clock is generated by the PHY, which uses a 25 MHz crystal for that.
This clock line is of course fed into the SoC/MAC, but also used (you might
say hijacked) by other chips on the board which depend on that clock being
_always_ on (at least after initial init on boot). The hardware can not be
changed, we speak of several hundred boards already sold in the last years.
O:-)
Symptom is: when calling `ip link set down dev eth0` that clock goes off, the
other (not soc nor phy) chips depending on that clock, freeze.
I could bisect this behaviour change on a vanilla kernel to the commit
mentioned above (actually to the backport commit v4.18.7-4-g716fc5ce90cf,
because I bisected from v4.17.19 to v4.18.20).
What I tracked down so far: macb_close() before the bugfix reset the MPE bit
in the MAC Network Control Register, which probably prevents the MAC to send
MDIO telegrams to the PHY? After the bugfix, that bit is not cleared anymore
(to allow still talking to other PHYs on the same MDIO bus, we don't have that
case). I assume communicating with the PHY is still possible then.
macb_close() also calls phy_stop() which sets the state of the phy driver
state machine to PHY_HALTED, with the next run of that state machine
phy_suspend() is called.
The smsc phy driver has no special suspend/resume functions, but uses
genphy_suspend(), that one sets BMCR_PDOWN in MII_BMCR register of that
(standard compliant) PHY. I suspect after that the PHY powers down and the
clock goes off.
I assume before that bugfix, this power down bit could not be set, because the
MDIO interface in the MAC had been disabled, so the PHY stayed on. (However
there's a possible race because in macb_close() the phy_stop() is called
before macb_reset_hw(), right?)
So far, these are mostly assumptions. I did not use gdb on the drivers or a
logic analyzer on the MDIO lines. I could do to prove, however.
What I could do:
1) Revert that change on my tree, which would mean reverting a generic bugfix
2) Patch smsc phy driver to not suspend anymore
3) Invent some new way to prevent suspend on a configuration basis (dt?)
4) Anything I did not think of yet
I know 1) or 2) are hacks without a chance to make it to mainline. What would
be your suggestions for 3) and 4)?
Greets
Alex
^ permalink raw reply
* Re: [PATCH net-next 01/16] qlge: Remove irq_cnt
From: Benjamin Poirier @ 2019-06-26 11:36 UTC (permalink / raw)
To: Manish Chopra; +Cc: GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <DM6PR18MB2697814343012B4363482290ABE20@DM6PR18MB2697.namprd18.prod.outlook.com>
On 2019/06/26 08:59, Manish Chopra wrote:
> > -----Original Message-----
> > From: Benjamin Poirier <bpoirier@suse.com>
> > Sent: Monday, June 17, 2019 1:19 PM
> > To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> > NIC-Dev@marvell.com>; netdev@vger.kernel.org
> > Subject: [PATCH net-next 01/16] qlge: Remove irq_cnt
> >
> > qlge uses an irq enable/disable refcounting scheme that is:
> > * poorly implemented
> > Uses a spin_lock to protect accesses to the irq_cnt atomic variable
> > * buggy
> > Breaks when there is not a 1:1 sequence of irq - napi_poll, such as
> > when using SO_BUSY_POLL.
> > * unnecessary
> > The purpose or irq_cnt is to reduce irq control writes when
> > multiple work items result from one irq: the irq is re-enabled
> > after all work is done.
> > Analysis of the irq handler shows that there is only one case where
> > there might be two workers scheduled at once, and those have
> > separate irq masking bits.
>
> I believe you are talking about here for asic error recovery worker and MPI worker.
> Which separate IRQ masking bits are you referring here ?
INTR_EN with intr_dis_mask for completion interrupts
INTR_MASK bit INTR_MASK_PI for mpi interrupts
> > static int ql_validate_flash(struct ql_adapter *qdev, u32 size, const char *str)
> > @@ -2500,21 +2451,22 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
> > u32 var;
> > int work_done = 0;
> >
> > - spin_lock(&qdev->hw_lock);
> > - if (atomic_read(&qdev->intr_context[0].irq_cnt)) {
> > - netif_printk(qdev, intr, KERN_DEBUG, qdev->ndev,
> > - "Shared Interrupt, Not ours!\n");
> > - spin_unlock(&qdev->hw_lock);
> > - return IRQ_NONE;
> > - }
> > - spin_unlock(&qdev->hw_lock);
> > + /* Experience shows that when using INTx interrupts, the device does
> > + * not always auto-mask the interrupt.
> > + * When using MSI mode, the interrupt must be explicitly disabled
> > + * (even though it is auto-masked), otherwise a later command to
> > + * enable it is not effective.
> > + */
> > + if (!test_bit(QL_MSIX_ENABLED, &qdev->flags))
> > + ql_disable_completion_interrupt(qdev, 0);
>
> Current code is disabling completion interrupt in case of MSI-X zeroth vector.
> This change will break that behavior. Shouldn't zeroth vector in case of MSI-X also disable completion interrupt ?
> If not, please explain why ?
In msix mode there's no need to explicitly disable completion
interrupts, they are reliably auto-masked, according to my observations.
I tested this on two QLE8142 adapters.
Do you have reason to believe this might not always be the case?
>
> >
> > - var = ql_disable_completion_interrupt(qdev, intr_context->intr);
> > + var = ql_read32(qdev, STS);
> >
> > /*
> > * Check for fatal error.
> > */
> > if (var & STS_FE) {
> > + ql_disable_completion_interrupt(qdev, 0);
>
> Why need to do it again here ? if before this it can disable completion interrupt for INT-X case and MSI-X zeroth vector case ?
I couldn't test this code path, so I preserved the original behavior.
>
> > ql_queue_asic_error(qdev);
> > netdev_err(qdev->ndev, "Got fatal error, STS = %x.\n", var);
> > var = ql_read32(qdev, ERR_STS);
> > @@ -2534,7 +2486,6 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
> > */
> > netif_err(qdev, intr, qdev->ndev,
> > "Got MPI processor interrupt.\n");
> > - ql_disable_completion_interrupt(qdev, intr_context->intr);
>
> Why disable interrupt is not required here ?
The interrupt source _is_ masked:
ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16));
> While in case of Fatal error case above ql_disable_completion_interrupt() is being called ?
> Also, in case of MSI-X zeroth vector it will not disable completion interrupt but at the end, it will end of qlge_isr() enabling completion interrupt.
> Seems like disabling and enabling might not be in sync in case of MSI-X zeroth vector.
I guess you forgot to consider that completion interrupts are
auto-masked in msix mode.
^ permalink raw reply
* KASAN: use-after-free Read in corrupted (3)
From: syzbot @ 2019-06-26 11:37 UTC (permalink / raw)
To: aarcange, akpm, christian, ebiederm, elena.reshetova, guro,
keescook, linux-kernel, luto, mhocko, mingo, namit, netdev,
peterz, riel, syzkaller-bugs, wad
Hello,
syzbot found the following crash on:
HEAD commit: 045df37e Merge branch 'cxgb4-Reference-count-MPS-TCAM-entr..
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=13c6217ea00000
kernel config: https://syzkaller.appspot.com/x/.config?x=dd16b8dc9d0d210c
dashboard link: https://syzkaller.appspot.com/bug?extid=8a821b383523654227bf
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1389f5b5a00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+8a821b383523654227bf@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in vsnprintf+0x1727/0x19a0 lib/vsprintf.c:2503
Read of size 8 at addr ffff8880952500a0 by task syz-executor.1/9180
CPU: 0 PID: 9180 Comm: syz-executor.1 Not tainted 5.2.0-rc5+ #43
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
Allocated by task 8:
save_stack+0x23/0x90 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_kmalloc mm/kasan/common.c:489 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:462
kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:497
slab_post_alloc_hook mm/slab.h:437 [inline]
slab_alloc mm/slab.c:3326 [inline]
kmem_cache_alloc+0x11a/0x6f0 mm/slab.c:3488
vm_area_dup+0x21/0x170 kernel/fork.c:343
dup_mmap kernel/fork.c:528 [inline]
dup_mm+0x8c4/0x13b0 kernel/fork.c:1341
copy_mm kernel/fork.c:1397 [inline]
copy_process.part.0+0x2cde/0x6790 kernel/fork.c:2032
copy_process kernel/fork.c:1800 [inline]
_do_fork+0x25d/0xfe0 kernel/fork.c:2369
__do_sys_clone kernel/fork.c:2476 [inline]
__se_sys_clone kernel/fork.c:2470 [inline]
__x64_sys_clone+0xbf/0x150 kernel/fork.c:2470
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 2502230480:
------------[ cut here ]------------
Bad or missing usercopy whitelist? Kernel memory overwrite attempt detected
to SLAB object 'shmem_inode_cache' (offset 1040, size 1)!
WARNING: CPU: 0 PID: 9180 at mm/usercopy.c:74 usercopy_warn+0xeb/0x110
mm/usercopy.c:74
Kernel panic - not syncing: panic_on_warn set ...
Shutting down cpus with NMI
Kernel Offset: disabled
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [EXT] [PATCH net-next 03/16] qlge: Deduplicate lbq_buf_size
From: Benjamin Poirier @ 2019-06-26 11:37 UTC (permalink / raw)
To: Manish Chopra; +Cc: GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <DM6PR18MB2697BAC4CA9B876306BEDBEBABE20@DM6PR18MB2697.namprd18.prod.outlook.com>
On 2019/06/26 09:24, Manish Chopra wrote:
> > -----Original Message-----
> > From: Benjamin Poirier <bpoirier@suse.com>
> > Sent: Monday, June 17, 2019 1:19 PM
> > To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> > NIC-Dev@marvell.com>; netdev@vger.kernel.org
> > Subject: [EXT] [PATCH net-next 03/16] qlge: Deduplicate lbq_buf_size
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > lbq_buf_size is duplicated to every rx_ring structure whereas lbq_buf_order is
> > present once in the ql_adapter structure. All rings use the same buf size, keep
> > only one copy of it. Also factor out the calculation of lbq_buf_size instead of
> > having two copies.
> >
> > Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> > ---
[...]
>
> Not sure if this change is really required, I think fields relevant to rx_ring should be present in the rx_ring structure.
> There are various other fields like "lbq_len" and "lbq_size" which would be same for all rx rings but still under the relevant rx_ring structure.
Indeed, those members are also removed by this patch series, in patch 11.
^ permalink raw reply
* Re: [EXT] [PATCH net-next 05/16] qlge: Remove rx_ring.sbq_buf_size
From: Benjamin Poirier @ 2019-06-26 11:39 UTC (permalink / raw)
To: Manish Chopra; +Cc: GR-Linux-NIC-Dev, netdev@vger.kernel.org
In-Reply-To: <DM6PR18MB269776CBA6B979855AD215A8ABE20@DM6PR18MB2697.namprd18.prod.outlook.com>
On 2019/06/26 09:36, Manish Chopra wrote:
> > -----Original Message-----
> > From: Benjamin Poirier <bpoirier@suse.com>
> > Sent: Monday, June 17, 2019 1:19 PM
> > To: Manish Chopra <manishc@marvell.com>; GR-Linux-NIC-Dev <GR-Linux-
> > NIC-Dev@marvell.com>; netdev@vger.kernel.org
> > Subject: [EXT] [PATCH net-next 05/16] qlge: Remove rx_ring.sbq_buf_size
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > Tx rings have sbq_buf_size = 0 but there's no case where the code actually
> > tests on that value. We can remove sbq_buf_size and use a constant instead.
> >
>
> Seems relevant to RX ring, not the TX ring ?
qlge uses "struct rx_ring" for rx and for tx completion rings.
The driver's author is probably laughing now at the success of his plan
to confuse those who would follow in his footsteps.
^ 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