Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 10/10] net: hns3: Prevent sending command during global or core reset
From: Salil Mehta @ 2018-07-06 10:28 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linuxarm, Huazhong Tan
In-Reply-To: <20180706102804.196-1-salil.mehta@huawei.com>

From: Huazhong Tan <tanhuazhong@huawei.com>

According to hardware's description, driver should not send command to
IMP while hardware doing global or core reset.

Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c  | 4 +++-
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 ++
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
index 82cf12a..eca4b23 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
@@ -206,7 +206,8 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc *desc, int num)
 
 	spin_lock_bh(&hw->cmq.csq.lock);
 
-	if (num > hclge_ring_space(&hw->cmq.csq)) {
+	if (num > hclge_ring_space(&hw->cmq.csq) ||
+	    test_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state)) {
 		spin_unlock_bh(&hw->cmq.csq.lock);
 		return -EBUSY;
 	}
@@ -346,6 +347,7 @@ int hclge_cmd_init(struct hclge_dev *hdev)
 	spin_lock_init(&hdev->hw.cmq.crq.lock);
 
 	hclge_cmd_init_regs(&hdev->hw);
+	clear_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
 
 	ret = hclge_cmd_query_firmware_version(&hdev->hw, &version);
 	if (ret) {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 4ca3e6b..8bbf4e5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2507,12 +2507,14 @@ static u32 hclge_check_event_cause(struct hclge_dev *hdev, u32 *clearval)
 
 	/* check for vector0 reset event sources */
 	if (BIT(HCLGE_VECTOR0_GLOBALRESET_INT_B) & rst_src_reg) {
+		set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
 		set_bit(HNAE3_GLOBAL_RESET, &hdev->reset_pending);
 		*clearval = BIT(HCLGE_VECTOR0_GLOBALRESET_INT_B);
 		return HCLGE_VECTOR0_EVENT_RST;
 	}
 
 	if (BIT(HCLGE_VECTOR0_CORERESET_INT_B) & rst_src_reg) {
+		set_bit(HCLGE_STATE_CMD_DISABLE, &hdev->state);
 		set_bit(HNAE3_CORE_RESET, &hdev->reset_pending);
 		*clearval = BIT(HCLGE_VECTOR0_CORERESET_INT_B);
 		return HCLGE_VECTOR0_EVENT_RST;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index 71d38b8..20abe82 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -128,6 +128,7 @@ enum HCLGE_DEV_STATE {
 	HCLGE_STATE_MBX_SERVICE_SCHED,
 	HCLGE_STATE_MBX_HANDLING,
 	HCLGE_STATE_STATISTICS_UPDATING,
+	HCLGE_STATE_CMD_DISABLE,
 	HCLGE_STATE_MAX
 };
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 08/10] net: hns3: Fix get_vector ops in hclgevf_main module
From: Salil Mehta @ 2018-07-06 10:28 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linuxarm, Yunsheng Lin
In-Reply-To: <20180706102804.196-1-salil.mehta@huawei.com>

From: Yunsheng Lin <linyunsheng@huawei.com>

The hclgevf_free_vector function expects the caller to pass
the vector_id to it, and hclgevf_put_vector pass vector to
it now, which will cause vector allocation problem.

This patch fixes it by converting vector into vector_id before
calling hclgevf_free_vector.

Fixes: e2cb1dec9779 ("net: hns3: Add HNS3 VF HCL(Hardware Compatibility Layer) Support")
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 5a86532..d1f16f0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -658,8 +658,17 @@ static int hclgevf_unmap_ring_from_vector(
 static int hclgevf_put_vector(struct hnae3_handle *handle, int vector)
 {
 	struct hclgevf_dev *hdev = hclgevf_ae_get_hdev(handle);
+	int vector_id;
+
+	vector_id = hclgevf_get_vector_index(hdev, vector);
+	if (vector_id < 0) {
+		dev_err(&handle->pdev->dev,
+			"hclgevf_put_vector get vector index fail. ret =%d\n",
+			vector_id);
+		return vector_id;
+	}
 
-	hclgevf_free_vector(hdev, vector);
+	hclgevf_free_vector(hdev, vector_id);
 
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related

* [PATCH net] ipfrag: really prevent allocation on netns exit
From: Paolo Abeni @ 2018-07-06 10:30 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Eric Dumazet

Setting the low threshold to 0 has no effect on frags allocation,
we need to clear high_thresh instead.

The code was pre-existent to commit 648700f76b03 ("inet: frags:
use rhashtables for reassembly units"), but before the above,
such assignment had a different role: prevent concurrent eviction
from the worker and the netns cleanup helper.

Fixes: 648700f76b03 ("inet: frags: use rhashtables for reassembly units")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/ipv4/inet_fragment.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index c9e35b81d093..1e4cf3ab560f 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -90,7 +90,7 @@ static void inet_frags_free_cb(void *ptr, void *arg)
 
 void inet_frags_exit_net(struct netns_frags *nf)
 {
-	nf->low_thresh = 0; /* prevent creation of new frags */
+	nf->high_thresh = 0; /* prevent creation of new frags */
 
 	rhashtable_free_and_destroy(&nf->rhashtable, inet_frags_free_cb, NULL);
 }
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 0/2] net/sched: act_skbedit: lockless data path
From: Davide Caratti @ 2018-07-06 10:40 UTC (permalink / raw)
  To: Cong Wang, David S. Miller; +Cc: netdev

the data path of act_skbedit can be faster if we avoid using spinlocks:
 - patch 1 converts act_skbedit statistics to use per-cpu counters
 - patch 2 lets act_skbedit use RCU to read/update its configuration 

test procedure (using pktgen from https://github.com/netoptimizer):

 # ip link add name eth1 type dummy
 # ip link set dev eth1 up
 # tc qdisc add dev eth1 clsact
 # tc filter add dev eth1 egress matchall action skbedit priority c1a0:c1a0
 # for c in 1 2 4 ; do
 > ./pktgen_bench_xmit_mode_queue_xmit.sh -v -s 64 -t $c -n 5000000 -i eth1
 > done

test results (avg. pps/thread)

  $c | before patch |  after patch | improvement
 ----+--------------+--------------+------------
   1 | 3917464 ± 3% | 4000458 ± 3% |  irrelevant
   2 | 3455367 ± 4% | 3953076 ± 1% |        +14%
   4 | 2496594 ± 2% | 3801123 ± 3% |        +52%


Davide Caratti (2):
  net/sched: skbedit: use per-cpu counters
  net/sched: act_skbedit: don't use spinlock in the data path

 include/net/tc_act/tc_skbedit.h |  37 ++++++++---
 net/sched/act_skbedit.c         | 113 ++++++++++++++++++++------------
 2 files changed, 98 insertions(+), 52 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH net-next 1/2] net/sched: skbedit: use per-cpu counters
From: Davide Caratti @ 2018-07-06 10:40 UTC (permalink / raw)
  To: Cong Wang, David S. Miller; +Cc: netdev
In-Reply-To: <cover.1530870765.git.dcaratti@redhat.com>

use per-CPU counters, instead of sharing a single set of stats with all
cores: this removes the need of spinlocks when stats are read/updated.

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/sched/act_skbedit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index dfaf5d8028dd..c1bfa28ba477 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -38,10 +38,10 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 {
 	struct tcf_skbedit *d = to_skbedit(a);
 
-	spin_lock(&d->tcf_lock);
 	tcf_lastuse_update(&d->tcf_tm);
-	bstats_update(&d->tcf_bstats, skb);
+	bstats_cpu_update(this_cpu_ptr(d->common.cpu_bstats), skb);
 
+	spin_lock(&d->tcf_lock);
 	if (d->flags & SKBEDIT_F_PRIORITY)
 		skb->priority = d->priority;
 	if (d->flags & SKBEDIT_F_INHERITDSFIELD) {
@@ -77,8 +77,8 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 	return d->tcf_action;
 
 err:
-	d->tcf_qstats.drops++;
 	spin_unlock(&d->tcf_lock);
+	qstats_drop_inc(this_cpu_ptr(d->common.cpu_qstats));
 	return TC_ACT_SHOT;
 }
 
@@ -163,7 +163,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 
 	if (!exists) {
 		ret = tcf_idr_create(tn, parm->index, est, a,
-				     &act_skbedit_ops, bind, false);
+				     &act_skbedit_ops, bind, true);
 		if (ret)
 			return ret;
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 2/2] net/sched: act_skbedit: don't use spinlock in the data path
From: Davide Caratti @ 2018-07-06 10:40 UTC (permalink / raw)
  To: Cong Wang, David S. Miller; +Cc: netdev
In-Reply-To: <cover.1530870765.git.dcaratti@redhat.com>

use RCU instead of spin_{,un}lock_bh, to protect concurrent read/write on
act_skbedit configuration. This reduces the effects of contention in the
data path, in case multiple readers are present.

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 include/net/tc_act/tc_skbedit.h |  37 ++++++++---
 net/sched/act_skbedit.c         | 107 ++++++++++++++++++++------------
 2 files changed, 95 insertions(+), 49 deletions(-)

diff --git a/include/net/tc_act/tc_skbedit.h b/include/net/tc_act/tc_skbedit.h
index 19cd3d345804..911bbac838a2 100644
--- a/include/net/tc_act/tc_skbedit.h
+++ b/include/net/tc_act/tc_skbedit.h
@@ -22,14 +22,19 @@
 #include <net/act_api.h>
 #include <linux/tc_act/tc_skbedit.h>
 
+struct tcf_skbedit_params {
+	u32 flags;
+	u32 priority;
+	u32 mark;
+	u32 mask;
+	u16 queue_mapping;
+	u16 ptype;
+	struct rcu_head rcu;
+};
+
 struct tcf_skbedit {
-	struct tc_action	common;
-	u32		flags;
-	u32		priority;
-	u32		mark;
-	u32		mask;
-	u16		queue_mapping;
-	u16		ptype;
+	struct tc_action common;
+	struct tcf_skbedit_params __rcu *params;
 };
 #define to_skbedit(a) ((struct tcf_skbedit *)a)
 
@@ -37,15 +42,27 @@ struct tcf_skbedit {
 static inline bool is_tcf_skbedit_mark(const struct tc_action *a)
 {
 #ifdef CONFIG_NET_CLS_ACT
-	if (a->ops && a->ops->type == TCA_ACT_SKBEDIT)
-		return to_skbedit(a)->flags == SKBEDIT_F_MARK;
+	u32 flags;
+
+	if (a->ops && a->ops->type == TCA_ACT_SKBEDIT) {
+		rcu_read_lock();
+		flags = rcu_dereference(to_skbedit(a)->params)->flags;
+		rcu_read_unlock();
+		return flags == SKBEDIT_F_MARK;
+	}
 #endif
 	return false;
 }
 
 static inline u32 tcf_skbedit_mark(const struct tc_action *a)
 {
-	return to_skbedit(a)->mark;
+	u32 mark;
+
+	rcu_read_lock();
+	mark = rcu_dereference(to_skbedit(a)->params)->mark;
+	rcu_read_unlock();
+
+	return mark;
 }
 
 #endif /* __NET_TC_SKBEDIT_H */
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index c1bfa28ba477..028eb679e532 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -37,14 +37,19 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 		       struct tcf_result *res)
 {
 	struct tcf_skbedit *d = to_skbedit(a);
+	struct tcf_skbedit_params *params;
+	int action;
 
 	tcf_lastuse_update(&d->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(d->common.cpu_bstats), skb);
 
-	spin_lock(&d->tcf_lock);
-	if (d->flags & SKBEDIT_F_PRIORITY)
-		skb->priority = d->priority;
-	if (d->flags & SKBEDIT_F_INHERITDSFIELD) {
+	rcu_read_lock();
+	params = rcu_dereference(d->params);
+	action = READ_ONCE(d->tcf_action);
+
+	if (params->flags & SKBEDIT_F_PRIORITY)
+		skb->priority = params->priority;
+	if (params->flags & SKBEDIT_F_INHERITDSFIELD) {
 		int wlen = skb_network_offset(skb);
 
 		switch (tc_skb_protocol(skb)) {
@@ -63,23 +68,23 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 			break;
 		}
 	}
-	if (d->flags & SKBEDIT_F_QUEUE_MAPPING &&
-	    skb->dev->real_num_tx_queues > d->queue_mapping)
-		skb_set_queue_mapping(skb, d->queue_mapping);
-	if (d->flags & SKBEDIT_F_MARK) {
-		skb->mark &= ~d->mask;
-		skb->mark |= d->mark & d->mask;
+	if (params->flags & SKBEDIT_F_QUEUE_MAPPING &&
+	    skb->dev->real_num_tx_queues > params->queue_mapping)
+		skb_set_queue_mapping(skb, params->queue_mapping);
+	if (params->flags & SKBEDIT_F_MARK) {
+		skb->mark &= ~params->mask;
+		skb->mark |= params->mark & params->mask;
 	}
-	if (d->flags & SKBEDIT_F_PTYPE)
-		skb->pkt_type = d->ptype;
-
-	spin_unlock(&d->tcf_lock);
-	return d->tcf_action;
+	if (params->flags & SKBEDIT_F_PTYPE)
+		skb->pkt_type = params->ptype;
 
+unlock:
+	rcu_read_unlock();
+	return action;
 err:
-	spin_unlock(&d->tcf_lock);
 	qstats_drop_inc(this_cpu_ptr(d->common.cpu_qstats));
-	return TC_ACT_SHOT;
+	action = TC_ACT_SHOT;
+	goto unlock;
 }
 
 static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
@@ -97,6 +102,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 			    int ovr, int bind, struct netlink_ext_ack *extack)
 {
 	struct tc_action_net *tn = net_generic(net, skbedit_net_id);
+	struct tcf_skbedit_params *params_old, *params_new;
 	struct nlattr *tb[TCA_SKBEDIT_MAX + 1];
 	struct tc_skbedit *parm;
 	struct tcf_skbedit *d;
@@ -176,25 +182,34 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 			return -EEXIST;
 	}
 
-	spin_lock_bh(&d->tcf_lock);
+	ASSERT_RTNL();
 
-	d->flags = flags;
+	params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
+	if (unlikely(!params_new)) {
+		if (ret == ACT_P_CREATED)
+			tcf_idr_release(*a, bind);
+		return -ENOMEM;
+	}
+
+	params_new->flags = flags;
 	if (flags & SKBEDIT_F_PRIORITY)
-		d->priority = *priority;
+		params_new->priority = *priority;
 	if (flags & SKBEDIT_F_QUEUE_MAPPING)
-		d->queue_mapping = *queue_mapping;
+		params_new->queue_mapping = *queue_mapping;
 	if (flags & SKBEDIT_F_MARK)
-		d->mark = *mark;
+		params_new->mark = *mark;
 	if (flags & SKBEDIT_F_PTYPE)
-		d->ptype = *ptype;
+		params_new->ptype = *ptype;
 	/* default behaviour is to use all the bits */
-	d->mask = 0xffffffff;
+	params_new->mask = 0xffffffff;
 	if (flags & SKBEDIT_F_MASK)
-		d->mask = *mask;
+		params_new->mask = *mask;
 
 	d->tcf_action = parm->action;
-
-	spin_unlock_bh(&d->tcf_lock);
+	params_old = rtnl_dereference(d->params);
+	rcu_assign_pointer(d->params, params_new);
+	if (params_old)
+		kfree_rcu(params_old, rcu);
 
 	if (ret == ACT_P_CREATED)
 		tcf_idr_insert(tn, *a);
@@ -206,33 +221,36 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_skbedit *d = to_skbedit(a);
+	struct tcf_skbedit_params *params;
 	struct tc_skbedit opt = {
 		.index   = d->tcf_index,
 		.refcnt  = d->tcf_refcnt - ref,
 		.bindcnt = d->tcf_bindcnt - bind,
 		.action  = d->tcf_action,
 	};
-	struct tcf_t t;
 	u64 pure_flags = 0;
+	struct tcf_t t;
+
+	params = rtnl_dereference(d->params);
 
 	if (nla_put(skb, TCA_SKBEDIT_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
-	if ((d->flags & SKBEDIT_F_PRIORITY) &&
-	    nla_put_u32(skb, TCA_SKBEDIT_PRIORITY, d->priority))
+	if ((params->flags & SKBEDIT_F_PRIORITY) &&
+	    nla_put_u32(skb, TCA_SKBEDIT_PRIORITY, params->priority))
 		goto nla_put_failure;
-	if ((d->flags & SKBEDIT_F_QUEUE_MAPPING) &&
-	    nla_put_u16(skb, TCA_SKBEDIT_QUEUE_MAPPING, d->queue_mapping))
+	if ((params->flags & SKBEDIT_F_QUEUE_MAPPING) &&
+	    nla_put_u16(skb, TCA_SKBEDIT_QUEUE_MAPPING, params->queue_mapping))
 		goto nla_put_failure;
-	if ((d->flags & SKBEDIT_F_MARK) &&
-	    nla_put_u32(skb, TCA_SKBEDIT_MARK, d->mark))
+	if ((params->flags & SKBEDIT_F_MARK) &&
+	    nla_put_u32(skb, TCA_SKBEDIT_MARK, params->mark))
 		goto nla_put_failure;
-	if ((d->flags & SKBEDIT_F_PTYPE) &&
-	    nla_put_u16(skb, TCA_SKBEDIT_PTYPE, d->ptype))
+	if ((params->flags & SKBEDIT_F_PTYPE) &&
+	    nla_put_u16(skb, TCA_SKBEDIT_PTYPE, params->ptype))
 		goto nla_put_failure;
-	if ((d->flags & SKBEDIT_F_MASK) &&
-	    nla_put_u32(skb, TCA_SKBEDIT_MASK, d->mask))
+	if ((params->flags & SKBEDIT_F_MASK) &&
+	    nla_put_u32(skb, TCA_SKBEDIT_MASK, params->mask))
 		goto nla_put_failure;
-	if (d->flags & SKBEDIT_F_INHERITDSFIELD)
+	if (params->flags & SKBEDIT_F_INHERITDSFIELD)
 		pure_flags |= SKBEDIT_F_INHERITDSFIELD;
 	if (pure_flags != 0 &&
 	    nla_put(skb, TCA_SKBEDIT_FLAGS, sizeof(pure_flags), &pure_flags))
@@ -248,6 +266,16 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
 	return -1;
 }
 
+static void tcf_skbedit_cleanup(struct tc_action *a)
+{
+	struct tcf_skbedit *d = to_skbedit(a);
+	struct tcf_skbedit_params *params;
+
+	params = rcu_dereference_protected(d->params, 1);
+	if (params)
+		kfree_rcu(params, rcu);
+}
+
 static int tcf_skbedit_walker(struct net *net, struct sk_buff *skb,
 			      struct netlink_callback *cb, int type,
 			      const struct tc_action_ops *ops,
@@ -273,6 +301,7 @@ static struct tc_action_ops act_skbedit_ops = {
 	.act		=	tcf_skbedit,
 	.dump		=	tcf_skbedit_dump,
 	.init		=	tcf_skbedit_init,
+	.cleanup	=	tcf_skbedit_cleanup,
 	.walk		=	tcf_skbedit_walker,
 	.lookup		=	tcf_skbedit_search,
 	.size		=	sizeof(struct tcf_skbedit),
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] 6lowpan: iphc: reset mac_header after decompress to fix panic
From: Marcel Holtmann @ 2018-07-06 10:46 UTC (permalink / raw)
  To: Michael Scott
  Cc: Alexander Aring, Jukka Rissanen, David S. Miller, linux-bluetooth,
	linux-wpan, netdev, linux-kernel
In-Reply-To: <20180619234406.8217-1-michael@opensourcefoundries.com>

Hi Michael,

> After decompression of 6lowpan socket data, an IPv6 header is inserted
> before the existing socket payload.  After this, we reset the
> network_header value of the skb to account for the difference in payload
> size from prior to decompression + the addition of the IPv6 header.
> 
> However, we fail to reset the mac_header value.
> 
> Leaving the mac_header value untouched here, can cause a calculation
> error in net/packet/af_packet.c packet_rcv() function when an
> AF_PACKET socket is opened in SOCK_RAW mode for use on a 6lowpan
> interface.
> 
> On line 2088, the data pointer is moved backward by the value returned
> from skb_mac_header().  If skb->data is adjusted so that it is before
> the skb->head pointer (which can happen when an old value of mac_header
> is left in place) the kernel generates a panic in net/core/skbuff.c
> line 1717.
> 
> This panic can be generated by BLE 6lowpan interfaces (such as bt0) and
> 802.15.4 interfaces (such as lowpan0) as they both use the same 6lowpan
> sources for compression and decompression.
> 
> Signed-off-by: Michael Scott <michael@opensourcefoundries.com>
> ---
> net/6lowpan/iphc.c | 1 +
> 1 file changed, 1 insertion(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel

^ permalink raw reply

* Re: [RFC PATCH] ip: re-introduce fragments cache worker
From: Eric Dumazet @ 2018-07-06 11:23 UTC (permalink / raw)
  To: Paolo Abeni, netdev
  Cc: David S. Miller, Eric Dumazet, Florian Westphal, NeilBrown
In-Reply-To: <6512d94713d40f1d572d2023168c48990f0d9cf0.1530798211.git.pabeni@redhat.com>



On 07/06/2018 03:10 AM, Paolo Abeni wrote:
> Currently, the ip frag cache is fragile to overload. With
> flow control disabled:
> 
> ./super_netperf.sh 10  -H 192.168.101.2 -t UDP_STREAM -l 60
> 9618.08
> ./super_netperf.sh 200  -H 192.168.101.2 -t UDP_STREAM -l 60
> 28.66
> 
> Once that the overload condition is reached, the system does not
> recover until it's almost completely idle:
> 
> ./super_netperf.sh 200  -H 192.168.101.2 -t UDP_STREAM -l 60 &
> sleep 4; I=0;
> for P in `pidof netperf`; do kill -9 $P; I=$((I+1)); [ $I -gt 190 ] && break; done
> 13.72
> 
> This is due to the removal of the fragment cache worker, which
> was responsible to free some IP fragment cache memory when the
> high threshold was reached, allowing the system to cope successfully
> with the next fragmented packets.
> 
> This commit re-introduces the worker, on a per netns basis. Thanks
> to rhashtable walkers we can block the bh only for an entry removal.
> 
> After this commit (and before IP frag worker removal):
> 
> ./super_netperf.sh 10  -H 192.168.101.2 -t UDP_STREAM -l 60
> 9618.08
> 
> ./super_netperf.sh 200  -H 192.168.101.2 -t UDP_STREAM -l 60
> 8599.77
> 
> ./super_netperf.sh 200  -H 192.168.101.2 -t UDP_STREAM -l 60 &
> sleep 4; I=0;
> for P in `pidof netperf`; do kill -9 $P; I=$((I+1)); [ $I -gt 190 ] && break; done
> 9623.12
> 
> Fixes: 648700f76b03 ("inet: frags: use rhashtables for reassembly units")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> Note: tweaking ipfrag sysfs does not solve completely the issue:
> - raising ipfrag_high_thresh increases the number of parallels
>   connections required to degrade the tput, but reached the IP
>   fragment cache capacity the good-put still goes almost to 0,
>   with the worker we get much more nice behaviour.
> - setting ipfrag_time to 2 increases the change to recover from
>   overload (the 2# test above), but with several experiments 
>   in such test, I got an average of 50% the expected tput with a very
>   large variance, with the worker we always see the expected/
>   line rate tput.
> ---

Ho hum. No please.

I do not think adding back a GC is wise, since my patches were going in the direction
of allowing us to increase limits on current hardware.

Meaning that the amount of frags to evict would be quite big under DDOS.
(One inet_frag_queue allocated for every incoming tiny frame :/ )

A GC is a _huge_ problem, burning one cpu (you would have to provision for this CPU)
compared to letting normal per frag timer doing its job.

My plan was to reduce the per frag timer under load (default is 30 seconds), since
this is exactly what your patch is indirectly doing, by aggressively pruning
frags under stress.

That would be a much simpler heuristic. [1]

BTW my own results (before patch) are :

lpaa5:/export/hda3/google/edumazet# ./super_netperf 10 -H 10.246.7.134 -t UDP_STREAM -l 60
   9602
lpaa5:/export/hda3/google/edumazet# ./super_netperf 200 -H 10.246.7.134 -t UDP_STREAM -l 60
   9557

On receiver (normal settings here) I had :

lpaa6:/export/hda3/google/edumazet# grep . /proc/sys/net/ipv4/ipfrag_*
/proc/sys/net/ipv4/ipfrag_high_thresh:104857600
/proc/sys/net/ipv4/ipfrag_low_thresh:78643200
/proc/sys/net/ipv4/ipfrag_max_dist:0
/proc/sys/net/ipv4/ipfrag_secret_interval:0
/proc/sys/net/ipv4/ipfrag_time:30

lpaa6:/export/hda3/google/edumazet# grep FRAG /proc/net/sockstat
FRAG: inuse 824 memory 53125312

[1] Something like (for IPv4 only here)

diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index c9e35b81d0931df8429a33e8d03e719b87da0747..88ed61bcda00f3357724e5c4dbcb97400b4a8b21 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -155,9 +155,15 @@ static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf,
                                               struct inet_frags *f,
                                               void *arg)
 {
+       long high_thresh = READ_ONCE(nf->high_thresh);
        struct inet_frag_queue *q;
+       u64 timeout;
+       long usage;
 
-       if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh)
+       if (!high_thresh)
+               return NULL;
+       usage = frag_mem_limit(nf);
+       if (usage > high_thresh)
                return NULL;
 
        q = kmem_cache_zalloc(f->frags_cachep, GFP_ATOMIC);
@@ -171,6 +177,8 @@ static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf,
        timer_setup(&q->timer, f->frag_expire, 0);
        spin_lock_init(&q->lock);
        refcount_set(&q->refcnt, 3);
+       timeout = (u64)nf->timeout * (high_thresh - usage);
+       mod_timer(&q->timer, jiffies + div64_long(timeout, high_thresh));
 
        return q;
 }
@@ -186,8 +194,6 @@ static struct inet_frag_queue *inet_frag_create(struct netns_frags *nf,
        if (!q)
                return NULL;
 
-       mod_timer(&q->timer, jiffies + nf->timeout);
-
        err = rhashtable_insert_fast(&nf->rhashtable, &q->node,
                                     f->rhash_params);
        if (err < 0) {

^ permalink raw reply related

* Re: [RFC PATCH] ip: re-introduce fragments cache worker
From: Paolo Abeni @ 2018-07-06 11:56 UTC (permalink / raw)
  To: Eric Dumazet, netdev
  Cc: David S. Miller, Eric Dumazet, Florian Westphal, NeilBrown
In-Reply-To: <51ef14ac-1d98-ad75-d282-eb6cb177fe7a@gmail.com>

Hi,

On Fri, 2018-07-06 at 04:23 -0700, Eric Dumazet wrote:
> Ho hum. No please.
> 
> I do not think adding back a GC is wise, since my patches were going in the direction
> of allowing us to increase limits on current hardware.
> 
> Meaning that the amount of frags to evict would be quite big under DDOS.
> (One inet_frag_queue allocated for every incoming tiny frame :/ )
> 
> A GC is a _huge_ problem, burning one cpu (you would have to provision for this CPU)
> compared to letting normal per frag timer doing its job.
> 
> My plan was to reduce the per frag timer under load (default is 30 seconds), since
> this is exactly what your patch is indirectly doing, by aggressively pruning
> frags under stress.
> 
> That would be a much simpler heuristic. [1]
> 
> BTW my own results (before patch) are :
> 
> lpaa5:/export/hda3/google/edumazet# ./super_netperf 10 -H 10.246.7.134 -t UDP_STREAM -l 60
>    9602
> lpaa5:/export/hda3/google/edumazet# ./super_netperf 200 -H 10.246.7.134 -t UDP_STREAM -l 60
>    9557
> 
> On receiver (normal settings here) I had :
> 
> lpaa6:/export/hda3/google/edumazet# grep . /proc/sys/net/ipv4/ipfrag_*
> /proc/sys/net/ipv4/ipfrag_high_thresh:104857600
> /proc/sys/net/ipv4/ipfrag_low_thresh:78643200
> /proc/sys/net/ipv4/ipfrag_max_dist:0
> /proc/sys/net/ipv4/ipfrag_secret_interval:0
> /proc/sys/net/ipv4/ipfrag_time:30
> 
> lpaa6:/export/hda3/google/edumazet# grep FRAG /proc/net/sockstat
> FRAG: inuse 824 memory 53125312

Than you for the feedback.

With your setting, you need a bit more concurrent connections (400 ?)
to saturate the ipfrag cache. Above that number, performances will
still sink.

> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index c9e35b81d0931df8429a33e8d03e719b87da0747..88ed61bcda00f3357724e5c4dbcb97400b4a8b21 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -155,9 +155,15 @@ static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf,
>                                                struct inet_frags *f,
>                                                void *arg)
>  {
> +       long high_thresh = READ_ONCE(nf->high_thresh);
>         struct inet_frag_queue *q;
> +       u64 timeout;
> +       long usage;
>  
> -       if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh)
> +       if (!high_thresh)
> +               return NULL;
> +       usage = frag_mem_limit(nf);
> +       if (usage > high_thresh)
>                 return NULL;
>  
>         q = kmem_cache_zalloc(f->frags_cachep, GFP_ATOMIC);
> @@ -171,6 +177,8 @@ static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf,
>         timer_setup(&q->timer, f->frag_expire, 0);
>         spin_lock_init(&q->lock);
>         refcount_set(&q->refcnt, 3);
> +       timeout = (u64)nf->timeout * (high_thresh - usage);
> +       mod_timer(&q->timer, jiffies + div64_long(timeout, high_thresh));
>  
>         return q;
>  }

This looks nice, I'll try to test it in my use case and I'll report
here.

Perhaps we can use the default timeout when usage < low_thresh, to
avoid some maths in possibly common scenario?

I have doubt: under DDOS we will trigger <max numfrags> timeout per
jiffy, can that keep a CPU busy, too?

Cheers,

Paolo

^ permalink raw reply

* Re: [PATCH] cfg80211: use IDA to allocate wiphy indeces
From: Johannes Berg @ 2018-07-06 11:57 UTC (permalink / raw)
  To: Brian Norris; +Cc: linux-kernel, linux-wireless, netdev, Ben Greear
In-Reply-To: <20180629184847.GA251207@ban.mtv.corp.google.com>

Hi Brian,

> > Imagine you have some userspace process running that has remembered the
> > wiphy index to use it to talk to nl80211, and now underneath the device
> > goes away and reappears. This process should understand that situation,
> > and handle it accordingly, rather than being blind to the reset.
> 
> How is this different from the wlan (netdev) device naming? We allow
> 'wlan0' to leave and return under the same name. Isn't the right answer
> that user space should be listening for udev and/or netlink events?

Well, first of all - for netdev *naming* these things differ in that
even if you get "wlan0" back, it will in fact have a new interface index
which hasn't been used before. So tools that are not aware of changes
since they don't listen will (hopefully) look up the interface index by
name once, and then keep using that, and then get failures on the
renames.

This doesn't even have to be all that long-running btw, it could be you
enter "iw wlan0 scan" and somewhere between looking up the wlan0
interface index and actually trying to do an operation on it your hw
crashes and the interface goes way. Or similar.

Now, with phy0 there's an additional limitation in that we made it so
you could only use "phyX" for X == phy index. This wasn't there
originally, and technically isn't really needed, but there are
races/issues with this.

In commit 7623225f90526, which really is a revert of Ben's patch that
always used the lowest number for the phy *name*. It looks like after I
had to revert that patch, Ben decided to just name them "wiphyX" with a
low number X in userspace, which is obviously fine.

I think the way to satisfy all of the different concerns around this
would be to track - separately - which phyX *names* (are going to) exist
in the system. As commit 7623225f90526 pointed out:

    This reverts commit 5a254ffe3ffdfa84fe076009bd8e88da412180d2.
    
    The commit failed to take into account that allocated wireless devices
    (wiphys) are not added into the device list upon allocation, but only
    when they are registered. Therefore, it opened up a race between
    allocating and registering a name, so that if two processes allocate and
    register concurrently ("alloc, alloc, register, register" rather than
    "alloc, register, alloc, register") the code will attempt to use the
    same name twice.


The IDA code you wrote avoids this situation because you add the wiphy
index to the IDA data structure on *allocation*, vs. relying just on the
regular rdev list like in Ben's commit.

So, to address my concerns about not reusing the number, I think we
could just decouple the phyX from the wiphy index X (iw has some magic
"phy#x" to use the actual wiphy index if you need to).

Then we can use the IDA to track the allocated *names*, and keep the
actual underlying *index* the same as today - similar to what you
observe with netdevs, e.g. wlan0.

The only complexity is that you have to track this when wiphys are being
renamed, both on renaming away from "phyX" (to free the name index X),
but also on renaming *to* "phyX" to reserve the name index X and fail
the rename if it's already reserved even though the name doesn't show up
on the output of "iw list" yet because it's not registered yet.

johannes

^ permalink raw reply

* Re: [bug report] net: ipv4: listified version of ip_rcv
From: Edward Cree @ 2018-07-06 12:09 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: netfilter-devel, netdev
In-Reply-To: <20180706083422.5ddtaxjxhnogcvgn@kili.mountain>

On 06/07/18 09:34, Dan Carpenter wrote:
> Hello Edward Cree,
>
> The patch 7da517a3bc52: "net: core: Another step of skb receive list
> processing" from Jul 2, 2018, leads to the following static checker
> warning:
>
> 	net/core/dev.c:5001 netif_receive_skb_list_internal()
> 	warn: 'skb' was already freed.
>
> The patch 17266ee93984: "net: ipv4: listified version of ip_rcv" from
> Jul 2, 2018, leads to the following static checker warning:
>
> 	./include/linux/netfilter.h:301 NF_HOOK_LIST()
> 	warn: 'skb' was already freed.
>
> [...]
>
> do_xdp_generic() frees skb.
>
> [...]
>
> For this one Smatch thinks that nf_hook() sometimes frees skb, but the
> code is less clear to me than for the previous warning so I don't know.
Yep, I think both warnings are correct.  Looks like changing from a dequeue-
 enqueue model to in-place list mangling just broke a bunch of assumptions
 I'd made in earlier versions of the patches.
Fixes coming soon.

Thanks,
-Ed

^ permalink raw reply

* Re: [RFC PATCH] ip: re-introduce fragments cache worker
From: Eric Dumazet @ 2018-07-06 12:09 UTC (permalink / raw)
  To: Paolo Abeni, Eric Dumazet, netdev
  Cc: David S. Miller, Eric Dumazet, Florian Westphal, NeilBrown
In-Reply-To: <b796ac9be31c52aa61c0936e6845af6e679a966e.camel@redhat.com>



On 07/06/2018 04:56 AM, Paolo Abeni wrote:
> Hi,
> 
> On Fri, 2018-07-06 at 04:23 -0700, Eric Dumazet wrote:
>> Ho hum. No please.
>>
>> I do not think adding back a GC is wise, since my patches were going in the direction
>> of allowing us to increase limits on current hardware.
>>
>> Meaning that the amount of frags to evict would be quite big under DDOS.
>> (One inet_frag_queue allocated for every incoming tiny frame :/ )
>>
>> A GC is a _huge_ problem, burning one cpu (you would have to provision for this CPU)
>> compared to letting normal per frag timer doing its job.
>>
>> My plan was to reduce the per frag timer under load (default is 30 seconds), since
>> this is exactly what your patch is indirectly doing, by aggressively pruning
>> frags under stress.
>>
>> That would be a much simpler heuristic. [1]
>>
>> BTW my own results (before patch) are :
>>
>> lpaa5:/export/hda3/google/edumazet# ./super_netperf 10 -H 10.246.7.134 -t UDP_STREAM -l 60
>>    9602
>> lpaa5:/export/hda3/google/edumazet# ./super_netperf 200 -H 10.246.7.134 -t UDP_STREAM -l 60
>>    9557
>>
>> On receiver (normal settings here) I had :
>>
>> lpaa6:/export/hda3/google/edumazet# grep . /proc/sys/net/ipv4/ipfrag_*
>> /proc/sys/net/ipv4/ipfrag_high_thresh:104857600
>> /proc/sys/net/ipv4/ipfrag_low_thresh:78643200
>> /proc/sys/net/ipv4/ipfrag_max_dist:0
>> /proc/sys/net/ipv4/ipfrag_secret_interval:0
>> /proc/sys/net/ipv4/ipfrag_time:30
>>
>> lpaa6:/export/hda3/google/edumazet# grep FRAG /proc/net/sockstat
>> FRAG: inuse 824 memory 53125312
> 
> Than you for the feedback.
> 
> With your setting, you need a bit more concurrent connections (400 ?)
> to saturate the ipfrag cache. Above that number, performances will
> still sink.

Maybe, but IP defrag can not be 'perfect'.

For this particular use case I could still bump high_thresh to 6 GB and all would be good :)

> This looks nice, I'll try to test it in my use case and I'll report
> here.
> 
> Perhaps we can use the default timeout when usage < low_thresh, to
> avoid some maths in possibly common scenario?

On current 64bit hardware, a divide here is not a big cost (compared to the rest
of frag setup)
and I would rather starting having smaller timeouts sooner than later ;)

(low_thresh is typically set to 75% of high_thresh)

> 
> I have doubt: under DDOS we will trigger <max numfrags> timeout per
> jiffy, can that keep a CPU busy, too?

Yes, the cpu(s) handling the RX queue(s), which are already provisioned for networking stuff ;)

Even without any frag being received, these cpu can be 100% busy.

^ permalink raw reply

* Re: [PATCH] wireless: mark expected switch fall-throughs
From: Johannes Berg @ 2018-07-06 12:29 UTC (permalink / raw)
  To: Gustavo A. R. Silva, David S. Miller; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20180704210553.GA7869@embeddedor.com>

Hi Gustavo,

> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.

You dropped the remark saying you didn't review them, but did you?

>  	case NL80211_CHAN_WIDTH_20:
>  		if (!ht_cap->ht_supported)
>  			return false;
> +		/* else: fall through */

What's the point in else:?

We also don't necessarily write

if (!...)
  return false;
else
  do_something();

but rather

if (!...)
  return false;
do_something().

I think I'd prefer without the "else:"

johannes

^ permalink raw reply

* [PATCH net-next] netfilter: fix NETFILTER_XT_TARGET_TEE dependencies
From: Arnd Bergmann @ 2018-07-06 12:37 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S. Miller
  Cc: Arnd Bergmann, Máté Eckl, Fernando Fernandez Mancera,
	Pablo M. Bermudo Garay, Felix Fietkau, netfilter-devel, coreteam,
	netdev, linux-kernel

With NETFILTER_XT_TARGET_TEE=y and IP6_NF_IPTABLES=m, we get a link
error when referencing the NF_DUP_IPV6 module:

net/netfilter/xt_TEE.o: In function `tee_tg6':
xt_TEE.c:(.text+0x14): undefined reference to `nf_dup_ipv6'

The problem here is the 'select NF_DUP_IPV6 if IP6_NF_IPTABLES'
that forces NF_DUP_IPV6 to be =m as well rather than setting it
to =y as was intended here. Adding a soft dependency on
IP6_NF_IPTABLES avoids that broken configuration.

Fixes: 35bf1ccecaaa ("netfilter: Kconfig: Change IPv6 select dependencies")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/netfilter/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 8aa4883c176a..e42c38c99741 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -985,7 +985,8 @@ config NETFILTER_XT_TARGET_TEE
 	tristate '"TEE" - packet cloning to alternate destination'
 	depends on NETFILTER_ADVANCED
 	depends on IPV6 || IPV6=n
-	depends on !NF_CONNTRACK || NF_CONNTRACK
+	depends on NF_CONNTRACK || !NF_CONNTRACK
+	depends on IP6_NF_IPTABLES || !IP6_NF_IPTABLES
 	select NF_DUP_IPV4
 	select NF_DUP_IPV6 if IP6_NF_IPTABLES
 	---help---
-- 
2.9.0

^ permalink raw reply related

* [PATCH net] net: bridge: fix br_vlan_get_{pvid,info} return values
From: Arnd Bergmann @ 2018-07-06 12:44 UTC (permalink / raw)
  To: David S. Miller
  Cc: Arnd Bergmann, Nikolay Aleksandrov, Jiri Pirko, Yotam Gigi,
	Roopa Prabhu, Petr Machata, netdev, linux-kernel

These two functions return the regular -EINVAL failure in the normal
code path, but return a nonstandard '-1' error otherwise, which gets
interpreted as -EPERM.

Let's change it to -EINVAL for the dummy functions as well.

Fixes: 4d4fd36126d6 ("net: bridge: Publish bridge accessor functions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/if_bridge.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 7843b98e1c6e..c20c7e197d07 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -105,13 +105,13 @@ static inline bool br_vlan_enabled(const struct net_device *dev)
 
 static inline int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
 {
-	return -1;
+	return -EINVAL;
 }
 
 static inline int br_vlan_get_info(const struct net_device *dev, u16 vid,
 				   struct bridge_vlan_info *p_vinfo)
 {
-	return -1;
+	return -EINVAL;
 }
 #endif
 
-- 
2.9.0

^ permalink raw reply related

* [PATCH net-next] mlxsw: spectrum_router: avoid uninitialized variable access
From: Arnd Bergmann @ 2018-07-06 12:44 UTC (permalink / raw)
  To: Jiri Pirko, Ido Schimmel, David S. Miller
  Cc: Arnd Bergmann, Petr Machata, Arkadi Sharshevsky, David Ahern,
	netdev, linux-kernel

When CONFIG_BRIDGE_VLAN_FILTERING is disabled, gcc correctly points out
that the 'vid' variable is uninitialized whenever br_vlan_get_pvid
returns an error:

drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c: In function 'mlxsw_sp_rif_vlan_fid_get':
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:6881:6: error: 'vid' may be used uninitialized in this function [-Werror=maybe-uninitialized]

This changes the condition check to always return -EINVAL here,
which I guess is what the author intended here.

Fixes: e6f1960ae6c7 ("mlxsw: spectrum_router: Allocate FID according to PVID")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 071428227b91..60c352ddd6fd 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -6878,11 +6878,9 @@ mlxsw_sp_rif_vlan_fid_get(struct mlxsw_sp_rif *rif,
 		vid = vlan_dev_vlan_id(rif->dev);
 	} else {
 		err = br_vlan_get_pvid(rif->dev, &vid);
-		if (!vid)
-			err = -EINVAL;
-		if (err) {
+		if (err < 0 || !vid) {
 			NL_SET_ERR_MSG_MOD(extack, "Couldn't determine bridge PVID");
-			return ERR_PTR(err);
+			return ERR_PTR(-EINVAL);
 		}
 	}
 
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH net-next] netfilter: fix NETFILTER_XT_TARGET_TEE dependencies
From: Pablo Neira Ayuso @ 2018-07-06 12:45 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jozsef Kadlecsik, Florian Westphal, David S. Miller,
	Máté Eckl, Fernando Fernandez Mancera,
	Pablo M. Bermudo Garay, Felix Fietkau, netfilter-devel, coreteam,
	netdev, linux-kernel
In-Reply-To: <20180706123805.2852900-1-arnd@arndb.de>

On Fri, Jul 06, 2018 at 02:37:58PM +0200, Arnd Bergmann wrote:
> With NETFILTER_XT_TARGET_TEE=y and IP6_NF_IPTABLES=m, we get a link
> error when referencing the NF_DUP_IPV6 module:
> 
> net/netfilter/xt_TEE.o: In function `tee_tg6':
> xt_TEE.c:(.text+0x14): undefined reference to `nf_dup_ipv6'
> 
> The problem here is the 'select NF_DUP_IPV6 if IP6_NF_IPTABLES'
> that forces NF_DUP_IPV6 to be =m as well rather than setting it
> to =y as was intended here. Adding a soft dependency on
> IP6_NF_IPTABLES avoids that broken configuration.

Applied to nf.git, thanks Arnd.

^ permalink raw reply

* Re: [PATCH net-next] netfilter: fix NETFILTER_XT_TARGET_TEE dependencies
From: Pablo Neira Ayuso @ 2018-07-06 12:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jozsef Kadlecsik, Florian Westphal, David S. Miller,
	Máté Eckl, Fernando Fernandez Mancera,
	Pablo M. Bermudo Garay, Felix Fietkau, netfilter-devel, coreteam,
	netdev, linux-kernel
In-Reply-To: <20180706124542.jybscbmzixyt4rx2@salvia>

On Fri, Jul 06, 2018 at 02:45:42PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jul 06, 2018 at 02:37:58PM +0200, Arnd Bergmann wrote:
> > With NETFILTER_XT_TARGET_TEE=y and IP6_NF_IPTABLES=m, we get a link
> > error when referencing the NF_DUP_IPV6 module:
> > 
> > net/netfilter/xt_TEE.o: In function `tee_tg6':
> > xt_TEE.c:(.text+0x14): undefined reference to `nf_dup_ipv6'
> > 
> > The problem here is the 'select NF_DUP_IPV6 if IP6_NF_IPTABLES'
> > that forces NF_DUP_IPV6 to be =m as well rather than setting it
> > to =y as was intended here. Adding a soft dependency on
> > IP6_NF_IPTABLES avoids that broken configuration.
> 
> Applied to nf.git, thanks Arnd.

Sorry, let me reconsider.

We many need similar patches to other spots in 35bf1ccecaaa ?

If so, it would be good to fix in the one go.

Thanks!

^ permalink raw reply

* Re: [PATCH net] net: bridge: fix br_vlan_get_{pvid,info} return values
From: Nikolay Aleksandrov @ 2018-07-06 12:51 UTC (permalink / raw)
  To: Arnd Bergmann, David S. Miller
  Cc: Jiri Pirko, Yotam Gigi, Roopa Prabhu, Petr Machata, netdev,
	linux-kernel
In-Reply-To: <20180706124420.3129099-1-arnd@arndb.de>

On 06/07/18 15:44, Arnd Bergmann wrote:
> These two functions return the regular -EINVAL failure in the normal
> code path, but return a nonstandard '-1' error otherwise, which gets
> interpreted as -EPERM.
> 
> Let's change it to -EINVAL for the dummy functions as well.
> 
> Fixes: 4d4fd36126d6 ("net: bridge: Publish bridge accessor functions")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/linux/if_bridge.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 7843b98e1c6e..c20c7e197d07 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -105,13 +105,13 @@ static inline bool br_vlan_enabled(const struct net_device *dev)
>  
>  static inline int br_vlan_get_pvid(const struct net_device *dev, u16 *p_pvid)
>  {
> -	return -1;
> +	return -EINVAL;
>  }
>  
>  static inline int br_vlan_get_info(const struct net_device *dev, u16 vid,
>  				   struct bridge_vlan_info *p_vinfo)
>  {
> -	return -1;
> +	return -EINVAL;
>  }
>  #endif
>  
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

^ permalink raw reply

* [PATCH] [net-next] devlink: fix incorrect return statement
From: Arnd Bergmann @ 2018-07-06 12:58 UTC (permalink / raw)
  To: Jiri Pirko, David S. Miller
  Cc: Arnd Bergmann, Arkadi Sharshevsky, Moshe Shemesh, David Ahern,
	netdev, linux-kernel

A newly added dummy helper function tries to return a failure from a "void"
function:

In file included from include/net/dsa.h:24,
                 from arch/arm/plat-orion/common.c:21:
include/net/devlink.h: In function 'devlink_param_value_changed':
include/net/devlink.h:771:9: error: 'return' with a value, in function returning void [-Werror]
  return -EOPNOTSUPP;

This fixes it by removing the bogus statement.

Fixes: ea601e170988 ("devlink: Add devlink notifications support for params")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/net/devlink.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8ed571385626..f67c29cede15 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -768,7 +768,6 @@ devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 static inline void
 devlink_param_value_changed(struct devlink *devlink, u32 param_id)
 {
-	return -EOPNOTSUPP;
 }
 
 #endif
-- 
2.9.0

^ permalink raw reply related

* [PATCH] netfilter: conntrack: add weak IPV6 dependency
From: Arnd Bergmann @ 2018-07-06 12:59 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S. Miller
  Cc: Arnd Bergmann, Máté Eckl, Fernando Fernandez Mancera,
	Pablo M. Bermudo Garay, Felix Fietkau, netfilter-devel, coreteam,
	netdev, linux-kernel

Now that the conntrack module contains code for ipv6, we can no longer
have it built-in while IPv6 itself is a loadable module:

net/netfilter/nf_conntrack_proto.o: In function `nf_ct_netns_do_get':
nf_conntrack_proto.c:(.text+0x88c): undefined reference to `nf_defrag_ipv6_enable'
net/netfilter/nf_conntrack_proto.o:(.rodata+0x178): undefined reference to `nf_conntrack_l4proto_icmpv6'

This adds a dependency on IPv6 that makes it possible to still build
the conntrack module with IPv6 disabled, but avoids the broken configuration.

Fixes: 66c524acfb51 ("netfilter: conntrack: remove l3proto abstraction")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/netfilter/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 74df382bf2ba..e42c38c99741 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -49,6 +49,7 @@ config NETFILTER_NETLINK_LOG
 config NF_CONNTRACK
 	tristate "Netfilter connection tracking support"
 	default m if NETFILTER_ADVANCED=n
+	depends on IPV6 || !IPV6
 	select NF_DEFRAG_IPV4
 	select NF_DEFRAG_IPV6 if IPV6
 	help
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH net-next] netfilter: fix NETFILTER_XT_TARGET_TEE dependencies
From: Arnd Bergmann @ 2018-07-06 13:05 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Jozsef Kadlecsik, Florian Westphal, David S. Miller,
	Máté Eckl, Fernando Fernandez Mancera,
	Pablo M. Bermudo Garay, Felix Fietkau, netfilter-devel, coreteam,
	Networking, Linux Kernel Mailing List
In-Reply-To: <20180706124827.xwho2rgu3jkk3hp5@salvia>

On Fri, Jul 6, 2018 at 2:48 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Jul 06, 2018 at 02:45:42PM +0200, Pablo Neira Ayuso wrote:
>> On Fri, Jul 06, 2018 at 02:37:58PM +0200, Arnd Bergmann wrote:
>> > With NETFILTER_XT_TARGET_TEE=y and IP6_NF_IPTABLES=m, we get a link
>> > error when referencing the NF_DUP_IPV6 module:
>> >
>> > net/netfilter/xt_TEE.o: In function `tee_tg6':
>> > xt_TEE.c:(.text+0x14): undefined reference to `nf_dup_ipv6'
>> >
>> > The problem here is the 'select NF_DUP_IPV6 if IP6_NF_IPTABLES'
>> > that forces NF_DUP_IPV6 to be =m as well rather than setting it
>> > to =y as was intended here. Adding a soft dependency on
>> > IP6_NF_IPTABLES avoids that broken configuration.
>>
>> Applied to nf.git, thanks Arnd.
>
> Sorry, let me reconsider.
>
> We many need similar patches to other spots in 35bf1ccecaaa ?
>
> If so, it would be good to fix in the one go.

I'm fairly sure that  NETFILTER_XT_TARGET_LOG is safe, since
NF_LOG_IPV6 already depends on IP6_NF_IPTABLES.

NF_SOCKET_IPV6 like NF_DUP_IPV6 does not depend on
IP6_NF_IPTABLES, so we may have a similar problem there, though
I have not come across that. I have done only a few hundred randconfig
builds since I started yesterday, so I may have missed something,
but I think this is safe because CONFIG_NF_SOCKET_IPV6 is
used like a 'bool' symbol these days, we don't actually control building
a module with it, only a small portion in the nft_socket file.

       Arnd

^ permalink raw reply

* [PATCH] ipv6: icmp: Updating pmtu for link local route
From: Georg Kohmann @ 2018-07-06 12:59 UTC (permalink / raw)
  To: netdev

When a ICMPV6_PKT_TOOBIG is received from a link local address the pmtu 
will be updated on a route with an arbitrary interface index. Subsequent 
packets sent back to the same link local address may therefore end up 
not considering the updated pmtu.

Current behavior breaks TAHI v6LC4.1.4 Reduce PMTU On-link.
Referring to RFC 1981: Section 3:
"Note that Path MTU Discovery must be performed even in cases where a 
node "thinks" a destination is attached to the same link as itself. In a 
situation such as when a neighboring router acts as proxy [ND] for some 
destination, the destination can to appear to be directly connected but 
is in fact more than one hop away."

Using the interface index from the incoming ICMPV6_PKT_TOOBIG when 
updating the pmtu.

Signed-off-by: Georg Kohmann <geokohma@cisco.com>
---
  net/ipv6/icmp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index be491bf..7d0e9c7 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -92,7 +92,7 @@ static void icmpv6_err(struct sk_buff *skb, struct 
inet6_skb_parm *opt,
         struct net *net = dev_net(skb->dev);

         if (type == ICMPV6_PKT_TOOBIG)
-               ip6_update_pmtu(skb, net, info, 0, 0, sock_net_uid(net, 
NULL));
+               ip6_update_pmtu(skb, net, info, skb->dev->ifindex, 0, 
sock_net_uid(net, NULL));
         else if (type == NDISC_REDIRECT)
                 ip6_redirect(skb, net, skb->dev->ifindex, 0,
                              sock_net_uid(net, NULL));
---

Tested on kernel 4.4.38.
I am however worried that this patch may have consequences for other 
traffic on the same interface. Please advice or suggest at better approach.

^ permalink raw reply related

* Re: [PATCH net-next] mlxsw: spectrum_router: avoid uninitialized variable access
From: Ido Schimmel @ 2018-07-06 13:16 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jiri Pirko, David S. Miller, Petr Machata, Arkadi Sharshevsky,
	David Ahern, netdev, linux-kernel
In-Reply-To: <20180706124455.3151925-1-arnd@arndb.de>

On Fri, Jul 06, 2018 at 02:44:45PM +0200, Arnd Bergmann wrote:
> When CONFIG_BRIDGE_VLAN_FILTERING is disabled, gcc correctly points out
> that the 'vid' variable is uninitialized whenever br_vlan_get_pvid
> returns an error:
> 
> drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c: In function 'mlxsw_sp_rif_vlan_fid_get':
> drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:6881:6: error: 'vid' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> This changes the condition check to always return -EINVAL here,
> which I guess is what the author intended here.
> 
> Fixes: e6f1960ae6c7 ("mlxsw: spectrum_router: Allocate FID according to PVID")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Ido Schimmel <idosch@mellanox.com>

Thanks!

^ permalink raw reply

* [PATCH net-next 0/6] Few improvements on mvneta
From: Gregory CLEMENT @ 2018-07-06 13:19 UTC (permalink / raw)
  To: David S. Miller, linux-kernel, netdev
  Cc: Andrew Lunn, Jason Cooper, Antoine Tenart, Gregory CLEMENT,
	Maxime Chevallier, Nadav Haklai, Yelena Krivosheev,
	Thomas Petazzoni, Miquèl Raynal, Marcin Wojtas,
	Dmitri Epshtein, linux-arm-kernel, Sebastian Hesselbarth

Hello,

This series brings some improvements for the mvneta driver.

The main one is the last patch which improves the driver for system
with low memory.

Patch 2 and 4 also contribute to improve the performance, whereas the
3 other ones are more about cleanup.

The series had been tested on ARM32 (clearfog) and ARM64
(EspressoBin), with and without hardware buffer management.

Gregory

Gregory CLEMENT (2):
  net: mvneta: remove data pointer usage from device_node structure
  net: mvneta: Allocate page for the descriptor

Yelena Krivosheev (4):
  net: mvneta: increase number of buffers in RX and TX queue
  net: mvneta: discriminate error cause for missed packet
  net: mvneta: Verify hardware checksum only when offload checksum
    feature is set
  net: mvneta: Improve the buffer allocation method for SWBM

 drivers/net/ethernet/marvell/mvneta.c    | 395 ++++++++++++++---------
 drivers/net/ethernet/marvell/mvneta_bm.c |  13 +
 drivers/net/ethernet/marvell/mvneta_bm.h |   8 +-
 3 files changed, 258 insertions(+), 158 deletions(-)

-- 
2.17.1

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox