Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net-next 5/6] net sched actions: fix misleading text strings in pedit action
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>

Change "tc filter pedit .." to "tc actions pedit .." in error
messages to clearly refer to pedit action.

Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 net/sched/act_pedit.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 3b775f54cee5..caa6927a992c 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -305,7 +305,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 
 			rc = pedit_skb_hdr_offset(skb, htype, &hoffset);
 			if (rc) {
-				pr_info("tc filter pedit bad header type specified (0x%x)\n",
+				pr_info("tc action pedit bad header type specified (0x%x)\n",
 					htype);
 				goto bad;
 			}
@@ -314,7 +314,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 				char *d, _d;
 
 				if (!offset_valid(skb, hoffset + tkey->at)) {
-					pr_info("tc filter pedit 'at' offset %d out of bounds\n",
+					pr_info("tc action pedit 'at' offset %d out of bounds\n",
 						hoffset + tkey->at);
 					goto bad;
 				}
@@ -326,12 +326,12 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 			}
 
 			if (offset % 4) {
-				pr_info("tc filter pedit offset must be on 32 bit boundaries\n");
+				pr_info("tc action pedit offset must be on 32 bit boundaries\n");
 				goto bad;
 			}
 
 			if (!offset_valid(skb, hoffset + offset)) {
-				pr_info("tc filter pedit offset %d out of bounds\n",
+				pr_info("tc action pedit offset %d out of bounds\n",
 					hoffset + offset);
 				goto bad;
 			}
@@ -349,7 +349,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 				val = (*ptr + tkey->val) & ~tkey->mask;
 				break;
 			default:
-				pr_info("tc filter pedit bad command (%d)\n",
+				pr_info("tc action pedit bad command (%d)\n",
 					cmd);
 				goto bad;
 			}
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 net-next 4/6] net sched actions: use sizeof operator for buffer length
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>

Replace constant integer with sizeof() to clearly indicate
the destination buffer length in skb_header_pointer() calls.

Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 net/sched/act_pedit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 9c2d8a31a5c5..3b775f54cee5 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -319,7 +319,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 					goto bad;
 				}
 				d = skb_header_pointer(skb, hoffset + tkey->at,
-						       1, &_d);
+						       sizeof(_d), &_d);
 				if (!d)
 					goto bad;
 				offset += (*d & tkey->offmask) >> tkey->shift;
@@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 			}
 
 			ptr = skb_header_pointer(skb, hoffset + offset,
-						 4, &hdata);
+						 sizeof(hdata), &hdata);
 			if (!ptr)
 				goto bad;
 			/* just do it, baby */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 net-next 3/6] net sched actions: fix sparse warning
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>

The variable _data in include/asm-generic/sections.h defines sections,
this causes sparse warning in pedit:

net/sched/act_pedit.c:293:35: warning: symbol '_data' shadows an earlier one
./include/asm-generic/sections.h:36:13: originally declared here

Therefore rename the variable.

Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 net/sched/act_pedit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index e4b29ee79ba8..9c2d8a31a5c5 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -290,7 +290,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 		enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
 		for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
-			u32 *ptr, _data;
+			u32 *ptr, hdata;
 			int offset = tkey->off;
 			int hoffset;
 			u32 val;
@@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 			}
 
 			ptr = skb_header_pointer(skb, hoffset + offset,
-						 4, &_data);
+						 4, &hdata);
 			if (!ptr)
 				goto bad;
 			/* just do it, baby */
@@ -355,7 +355,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 			}
 
 			*ptr = ((*ptr & tkey->mask) ^ val);
-			if (ptr == &_data)
+			if (ptr == &hdata)
 				skb_store_bits(skb, hoffset + offset, ptr, 4);
 		}
 
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v7 03/11] net: pch_gbe: Probe PHY ID & initialize only once
From: Andrew Lunn @ 2018-06-27 17:33 UTC (permalink / raw)
  To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627173106.s2lbolvz4x5mqr64@pburton-laptop>

> This is actually needed because pch_gbe_check_options() sets up, amongst
> other things, the autoneg_advertised field in struct pch_gbe_phy_info
> and that needs to happen before pch_gbe_phy_init_setting() is called.

Hi Paul

Please add a comment to the commit message about this.

       Andrew

^ permalink raw reply

* [PATCH v2 net-next 2/6] net sched actions: fix coding style in pedit headers
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>

Fix coding style issues in tc pedit headers detected by the
checkpatch script.

Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 include/net/tc_act/tc_pedit.h        | 1 +
 include/uapi/linux/tc_act/tc_pedit.h | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h
index 227a6f1d02f4..fac3ad4a86de 100644
--- a/include/net/tc_act/tc_pedit.h
+++ b/include/net/tc_act/tc_pedit.h
@@ -17,6 +17,7 @@ struct tcf_pedit {
 	struct tc_pedit_key	*tcfp_keys;
 	struct tcf_pedit_key_ex	*tcfp_keys_ex;
 };
+
 #define to_pedit(a) ((struct tcf_pedit *)a)
 
 static inline bool is_tcf_pedit(const struct tc_action *a)
diff --git a/include/uapi/linux/tc_act/tc_pedit.h b/include/uapi/linux/tc_act/tc_pedit.h
index 162d1094c41c..24ec792dacc1 100644
--- a/include/uapi/linux/tc_act/tc_pedit.h
+++ b/include/uapi/linux/tc_act/tc_pedit.h
@@ -17,13 +17,15 @@ enum {
 	TCA_PEDIT_KEY_EX,
 	__TCA_PEDIT_MAX
 };
+
 #define TCA_PEDIT_MAX (__TCA_PEDIT_MAX - 1)
-                                                                                
+
 enum {
 	TCA_PEDIT_KEY_EX_HTYPE = 1,
 	TCA_PEDIT_KEY_EX_CMD = 2,
 	__TCA_PEDIT_KEY_EX_MAX
 };
+
 #define TCA_PEDIT_KEY_EX_MAX (__TCA_PEDIT_KEY_EX_MAX - 1)
 
  /* TCA_PEDIT_KEY_EX_HDR_TYPE_NETWROK is a special case for legacy users. It
@@ -38,6 +40,7 @@ enum pedit_header_type {
 	TCA_PEDIT_KEY_EX_HDR_TYPE_UDP = 5,
 	__PEDIT_HDR_TYPE_MAX,
 };
+
 #define TCA_PEDIT_HDR_TYPE_MAX (__PEDIT_HDR_TYPE_MAX - 1)
 
 enum pedit_cmd {
@@ -45,6 +48,7 @@ enum pedit_cmd {
 	TCA_PEDIT_KEY_EX_CMD_ADD = 1,
 	__PEDIT_CMD_MAX,
 };
+
 #define TCA_PEDIT_CMD_MAX (__PEDIT_CMD_MAX - 1)
 
 struct tc_pedit_key {
@@ -55,13 +59,14 @@ struct tc_pedit_key {
 	__u32           offmask;
 	__u32           shift;
 };
-                                                                                
+
 struct tc_pedit_sel {
 	tc_gen;
 	unsigned char           nkeys;
 	unsigned char           flags;
 	struct tc_pedit_key     keys[0];
 };
+
 #define tc_pedit tc_pedit_sel
 
 #endif
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 net-next 1/6] net sched actions: fix coding style in pedit action
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak
In-Reply-To: <1530120815-13736-1-git-send-email-mrv@mojatatu.com>

Fix coding style issues in tc pedit action detected by the
checkpatch script.

Reviewed-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
 net/sched/act_pedit.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 8a925c72db5f..e4b29ee79ba8 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -136,15 +136,15 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 {
 	struct tc_action_net *tn = net_generic(net, pedit_net_id);
 	struct nlattr *tb[TCA_PEDIT_MAX + 1];
-	struct nlattr *pattr;
-	struct tc_pedit *parm;
-	int ret = 0, err;
-	struct tcf_pedit *p;
 	struct tc_pedit_key *keys = NULL;
 	struct tcf_pedit_key_ex *keys_ex;
+	struct tc_pedit *parm;
+	struct nlattr *pattr;
+	struct tcf_pedit *p;
+	int ret = 0, err;
 	int ksize;
 
-	if (nla == NULL)
+	if (!nla)
 		return -EINVAL;
 
 	err = nla_parse_nested(tb, TCA_PEDIT_MAX, nla, pedit_policy, NULL);
@@ -175,7 +175,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 			return ret;
 		p = to_pedit(*a);
 		keys = kmalloc(ksize, GFP_KERNEL);
-		if (keys == NULL) {
+		if (!keys) {
 			tcf_idr_release(*a, bind);
 			kfree(keys_ex);
 			return -ENOMEM;
@@ -220,6 +220,7 @@ static void tcf_pedit_cleanup(struct tc_action *a)
 {
 	struct tcf_pedit *p = to_pedit(a);
 	struct tc_pedit_key *keys = p->tcfp_keys;
+
 	kfree(keys);
 	kfree(p->tcfp_keys_ex);
 }
@@ -284,7 +285,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 	if (p->tcfp_nkeys > 0) {
 		struct tc_pedit_key *tkey = p->tcfp_keys;
 		struct tcf_pedit_key_ex *tkey_ex = p->tcfp_keys_ex;
-		enum pedit_header_type htype = TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
+		enum pedit_header_type htype =
+			TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK;
 		enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET;
 
 		for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
@@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 						hoffset + tkey->at);
 					goto bad;
 				}
-				d = skb_header_pointer(skb, hoffset + tkey->at, 1,
-						       &_d);
+				d = skb_header_pointer(skb, hoffset + tkey->at,
+						       1, &_d);
 				if (!d)
 					goto bad;
 				offset += (*d & tkey->offmask) >> tkey->shift;
 			}
 
 			if (offset % 4) {
-				pr_info("tc filter pedit"
-					" offset must be on 32 bit boundaries\n");
+				pr_info("tc filter pedit offset must be on 32 bit boundaries\n");
 				goto bad;
 			}
 
@@ -335,7 +336,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 				goto bad;
 			}
 
-			ptr = skb_header_pointer(skb, hoffset + offset, 4, &_data);
+			ptr = skb_header_pointer(skb, hoffset + offset,
+						 4, &_data);
 			if (!ptr)
 				goto bad;
 			/* just do it, baby */
@@ -358,8 +360,9 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
 		}
 
 		goto done;
-	} else
+	} else {
 		WARN(1, "pedit BUG: index %d\n", p->tcf_index);
+	}
 
 bad:
 	p->tcf_qstats.overlimits++;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v2 net-next 0/6] net sched actions: code style cleanup and fixes
From: Roman Mashak @ 2018-06-27 17:33 UTC (permalink / raw)
  To: davem; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri, Roman Mashak

The patchset fixes a few code stylistic issues and typos, as well as one
detected by sparse semantic checker tool.

No functional changes introduced.

Patch 1 & 2 fix coding style bits caught by the checkpatch.pl script
Patch 3 fixes an issue with a shadowed variable
Patch 4 adds sizeof() operator instead of magic number for buffer length
Patch 5 fixes typos in diagnostics messages
Patch 6 explicitly sets unsigned char for bitwise operation

v2:
   - submit for net-next
   - added Reviewed-by tags
   - use u8* instead of char* as per Davide Caratti suggestion

Roman Mashak (6):
  net sched actions: fix coding style in pedit action
  net sched actions: fix coding style in pedit headers
  net sched actions: fix sparse warning
  net sched actions: use sizeof operator for buffer length
  net sched actions: fix misleading text strings in pedit action
  net sched actions: avoid bitwise operation on signed value in pedit

 include/net/tc_act/tc_pedit.h        |  1 +
 include/uapi/linux/tc_act/tc_pedit.h |  9 ++++++--
 net/sched/act_pedit.c                | 43 +++++++++++++++++++-----------------
 3 files changed, 31 insertions(+), 22 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH net-next] netem: slotting with non-uniform distribution
From: Yousuk Seung @ 2018-06-27 17:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Yousuk Seung

Extend slotting with support for non-uniform distributions. This is
similar to netem's non-uniform distribution delay feature.

Commit f043efeae2f1 ("netem: support delivering packets in delayed
time slots") added the slotting feature to approximate the behaviors
of media with packet aggregation but only supported a uniform
distribution for delays between transmission attempts. Tests with TCP
BBR with emulated wifi links with non-uniform distributions produced
more useful results.

Syntax:
   slot dist DISTRIBUTION DELAY JITTER [packets MAX_PACKETS] \
      [bytes MAX_BYTES]

The syntax and use of the distribution table is the same as in the
non-uniform distribution delay feature. A file DISTRIBUTION must be
present in TC_LIB_DIR (e.g. /usr/lib/tc) containing numbers scaled by
NETEM_DIST_SCALE. A random value x is selected from the table and it
takes DELAY + ( x * JITTER ) as delay. Correlation between values is not
supported.

Examples:
  Normal distribution delay with mean = 800us and stdev = 100us.
  > tc qdisc add dev eth0 root netem slot dist normal 800us 100us

  Optionally set the max slot size in bytes and/or packets.
  > tc qdisc add dev eth0 root netem slot dist normal 800us 100us \
    bytes 64k packets 42

Signed-off-by: Yousuk Seung <ysseung@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
---
 include/uapi/linux/pkt_sched.h |  3 ++
 net/sched/sch_netem.c          | 73 +++++++++++++++++++++++-----------
 2 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 37b5096ae97b..bad3c03bcf43 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -539,6 +539,7 @@ enum {
 	TCA_NETEM_LATENCY64,
 	TCA_NETEM_JITTER64,
 	TCA_NETEM_SLOT,
+	TCA_NETEM_SLOT_DIST,
 	__TCA_NETEM_MAX,
 };
 
@@ -581,6 +582,8 @@ struct tc_netem_slot {
 	__s64   max_delay;
 	__s32   max_packets;
 	__s32   max_bytes;
+	__s64	dist_delay; /* nsec */
+	__s64	dist_jitter; /* nsec */
 };
 
 enum {
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 7d6801fc5340..ad18a2052416 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -68,6 +68,11 @@
 		 Fabio Ludovici <fabio.ludovici at yahoo.it>
 */
 
+struct disttable {
+	u32  size;
+	s16 table[0];
+};
+
 struct netem_sched_data {
 	/* internal t(ime)fifo qdisc uses t_root and sch->limit */
 	struct rb_root t_root;
@@ -99,10 +104,7 @@ struct netem_sched_data {
 		u32 rho;
 	} delay_cor, loss_cor, dup_cor, reorder_cor, corrupt_cor;
 
-	struct disttable {
-		u32  size;
-		s16 table[0];
-	} *delay_dist;
+	struct disttable *delay_dist;
 
 	enum  {
 		CLG_RANDOM,
@@ -142,6 +144,7 @@ struct netem_sched_data {
 		s32 bytes_left;
 	} slot;
 
+	struct disttable *slot_dist;
 };
 
 /* Time stamp put into socket buffer control block
@@ -180,7 +183,7 @@ static u32 get_crandom(struct crndstate *state)
 	u64 value, rho;
 	unsigned long answer;
 
-	if (state->rho == 0)	/* no correlation */
+	if (!state || state->rho == 0)	/* no correlation */
 		return prandom_u32();
 
 	value = prandom_u32();
@@ -601,10 +604,19 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 
 static void get_slot_next(struct netem_sched_data *q, u64 now)
 {
-	q->slot.slot_next = now + q->slot_config.min_delay +
-		(prandom_u32() *
-			(q->slot_config.max_delay -
-				q->slot_config.min_delay) >> 32);
+	s64 next_delay;
+
+	if (!q->slot_dist)
+		next_delay = q->slot_config.min_delay +
+				(prandom_u32() *
+				 (q->slot_config.max_delay -
+				  q->slot_config.min_delay) >> 32);
+	else
+		next_delay = tabledist(q->slot_config.dist_delay,
+				       (s32)(q->slot_config.dist_jitter),
+				       NULL, q->slot_dist);
+
+	q->slot.slot_next = now + next_delay;
 	q->slot.packets_left = q->slot_config.max_packets;
 	q->slot.bytes_left = q->slot_config.max_bytes;
 }
@@ -721,9 +733,9 @@ static void dist_free(struct disttable *d)
  * signed 16 bit values.
  */
 
-static int get_dist_table(struct Qdisc *sch, const struct nlattr *attr)
+static int get_dist_table(struct Qdisc *sch, struct disttable **tbl,
+			  const struct nlattr *attr)
 {
-	struct netem_sched_data *q = qdisc_priv(sch);
 	size_t n = nla_len(attr)/sizeof(__s16);
 	const __s16 *data = nla_data(attr);
 	spinlock_t *root_lock;
@@ -744,7 +756,7 @@ static int get_dist_table(struct Qdisc *sch, const struct nlattr *attr)
 	root_lock = qdisc_root_sleeping_lock(sch);
 
 	spin_lock_bh(root_lock);
-	swap(q->delay_dist, d);
+	swap(*tbl, d);
 	spin_unlock_bh(root_lock);
 
 	dist_free(d);
@@ -762,7 +774,8 @@ static void get_slot(struct netem_sched_data *q, const struct nlattr *attr)
 		q->slot_config.max_bytes = INT_MAX;
 	q->slot.packets_left = q->slot_config.max_packets;
 	q->slot.bytes_left = q->slot_config.max_bytes;
-	if (q->slot_config.min_delay | q->slot_config.max_delay)
+	if (q->slot_config.min_delay | q->slot_config.max_delay |
+	    q->slot_config.dist_jitter)
 		q->slot.slot_next = ktime_get_ns();
 	else
 		q->slot.slot_next = 0;
@@ -926,16 +939,17 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
 	}
 
 	if (tb[TCA_NETEM_DELAY_DIST]) {
-		ret = get_dist_table(sch, tb[TCA_NETEM_DELAY_DIST]);
-		if (ret) {
-			/* recover clg and loss_model, in case of
-			 * q->clg and q->loss_model were modified
-			 * in get_loss_clg()
-			 */
-			q->clg = old_clg;
-			q->loss_model = old_loss_model;
-			return ret;
-		}
+		ret = get_dist_table(sch, &q->delay_dist,
+				     tb[TCA_NETEM_DELAY_DIST]);
+		if (ret)
+			goto get_table_failure;
+	}
+
+	if (tb[TCA_NETEM_SLOT_DIST]) {
+		ret = get_dist_table(sch, &q->slot_dist,
+				     tb[TCA_NETEM_SLOT_DIST]);
+		if (ret)
+			goto get_table_failure;
 	}
 
 	sch->limit = qopt->limit;
@@ -983,6 +997,15 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
 		get_slot(q, tb[TCA_NETEM_SLOT]);
 
 	return ret;
+
+get_table_failure:
+	/* recover clg and loss_model, in case of
+	 * q->clg and q->loss_model were modified
+	 * in get_loss_clg()
+	 */
+	q->clg = old_clg;
+	q->loss_model = old_loss_model;
+	return ret;
 }
 
 static int netem_init(struct Qdisc *sch, struct nlattr *opt,
@@ -1011,6 +1034,7 @@ static void netem_destroy(struct Qdisc *sch)
 	if (q->qdisc)
 		qdisc_destroy(q->qdisc);
 	dist_free(q->delay_dist);
+	dist_free(q->slot_dist);
 }
 
 static int dump_loss_model(const struct netem_sched_data *q,
@@ -1127,7 +1151,8 @@ static int netem_dump(struct Qdisc *sch, struct sk_buff *skb)
 	if (dump_loss_model(q, skb) != 0)
 		goto nla_put_failure;
 
-	if (q->slot_config.min_delay | q->slot_config.max_delay) {
+	if (q->slot_config.min_delay | q->slot_config.max_delay |
+	    q->slot_config.dist_jitter) {
 		slot = q->slot_config;
 		if (slot.max_packets == INT_MAX)
 			slot.max_packets = 0;
-- 
2.18.0.rc2.346.g013aa6912e-goog

^ permalink raw reply related

* Re: [PATCH v7 03/11] net: pch_gbe: Probe PHY ID & initialize only once
From: Paul Burton @ 2018-06-27 17:31 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, David S . Miller
In-Reply-To: <20180627172131.GB885@lunn.ch>

Hi Andrew,

On Wed, Jun 27, 2018 at 07:21:31PM +0200, Andrew Lunn wrote:
> > [1] Please, someone patent PHY hotplugging & rigorously enforce said
> >     patent such that nobody can do it. At least not with an EG20T MAC.
> 
> Hi Paul
> 
> It is already possible, and probably patented. SFP cages are usually
> used for fibre optical modules. But it is also possible to have copper
> modules, which contain a standard PHY. And SFP modules are
> hot-plugable...

D'oh, but at least not relevant to the EG20T/pch_gbe :)

> > @@ -2577,6 +2579,8 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> >  	if (ret)
> >  		goto err_free_netdev;
> >  
> > +	pch_gbe_check_options(adapter);
> > +
> >  	/* Initialize PHY */
> >  	ret = pch_gbe_init_phy(adapter);
> >  	if (ret) {
> > @@ -2606,8 +2610,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> >  
> >  	INIT_WORK(&adapter->reset_task, pch_gbe_reset_task);
> >  
> > -	pch_gbe_check_options(adapter);
> > -
> >  	/* initialize the wol settings based on the eeprom settings */
> >  	adapter->wake_up_evt = PCH_GBE_WL_INIT_SETTING;
> >  	dev_info(&pdev->dev, "MAC address : %pM\n", netdev->dev_addr);
> 
> But these two changes seem unrelated. Should they be in a different
> patch?

This is actually needed because pch_gbe_check_options() sets up, amongst
other things, the autoneg_advertised field in struct pch_gbe_phy_info
and that needs to happen before pch_gbe_phy_init_setting() is called.

Thanks,
    Paul

^ permalink raw reply

* Re: [PATCH v7 07/11] net: pch_gbe: Remove AR8031 PHY hibernation disable
From: Andrew Lunn @ 2018-06-27 17:30 UTC (permalink / raw)
  To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-8-paul.burton@mips.com>

On Tue, Jun 26, 2018 at 05:06:08PM -0700, Paul Burton wrote:
> We should now be able to cope with the PHY entering hibernation, ie.
> ceasing to provide the RX clock, whilst the ethernet link is down.
> 
> Remove the code responsible for disabling the AR8031 PHY's hibernation
> feature, allowing the PHY to enter its low power hibernation state.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH v7 06/11] net: pch_gbe: Only enable MAC when PHY link is active
From: Andrew Lunn @ 2018-06-27 17:30 UTC (permalink / raw)
  To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-7-paul.burton@mips.com>

On Tue, Jun 26, 2018 at 05:06:07PM -0700, Paul Burton wrote:
> When using a PHY connected via RGMII, as the pch_gbe driver presumes is
> the case, the RX clock is provided by the PHY to the MAC. Various PHYs,
> including both the AR8031 used by the Minnowboard & the RTL8211E used by
> the MIPS Boston development board, will stop generating the RX clock
> when the ethernet link is down (eg. the ethernet cable is unplugged).
> 
> Various pieces of functionality in the EG20T MAC, ranging from basics
> like completing a MAC reset to programming MAC addresses, rely upon the
> RX clock being provided. When the clock is not provided these pieces of
> functionality simply never complete, and the busy bits that indicate
> they're in progress remain set indefinitely.
> 
> The pch_gbe driver currently requires that the RX clock is always
> provided, and attempts to enforce this by disabling the hibernation
> feature of the AR8031 PHY to keep it generating the RX clock. This patch
> moves us away from this model by only configuring the MAC when the PHY
> indicates that the ethernet link is up. When the link is up we should be
> able to safely expect that the RX clock is being provided, and therefore
> safely reset & configure the MAC.

Hi Paul

I like the concept, but the implementation is not clear. Maybe it just
needs more details in the commit message. What has the watchdog got to
do with link up?

And what happens on link down? Does the MAC need shutting down? I
don't see such code here.

    Andrew

^ permalink raw reply

* Re: [PATCH v7 05/11] net: pch_gbe: Move pch_gbe_watchdog lower in pch_gbe_main.c
From: Andrew Lunn @ 2018-06-27 17:23 UTC (permalink / raw)
  To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-6-paul.burton@mips.com>

On Tue, Jun 26, 2018 at 05:06:06PM -0700, Paul Burton wrote:
> This patch moves the pch_gbe_watchdog() function lower in pch_gbe_main.c
> in order to allow use of other functions in the next patch, without
> requiring lots of forward declarations. Doing this as a separate patch
> makes it clearer what actually changed in the next patch.
> 
> The function is unmodified except for whitespace changes to satisfy
> checkpatch.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH v7 03/11] net: pch_gbe: Probe PHY ID & initialize only once
From: Andrew Lunn @ 2018-06-27 17:21 UTC (permalink / raw)
  To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-4-paul.burton@mips.com>

On Tue, Jun 26, 2018 at 05:06:04PM -0700, Paul Burton wrote:
> The pch_gbe driver currently probes for the PHY ID & configures the PHY
> every time the MAC is reset, even though we know that the PHY won't have
> changed since the last MAC reset [1].
> 
> This patch moves the PHY probe to instead happen only once when the
> driver is probed, saving time & moving us closer to the behavior we'll
> have with phylib.
> 
> [1] Please, someone patent PHY hotplugging & rigorously enforce said
>     patent such that nobody can do it. At least not with an EG20T MAC.

Hi Paul

It is already possible, and probably patented. SFP cages are usually
used for fibre optical modules. But it is also possible to have copper
modules, which contain a standard PHY. And SFP modules are
hot-plugable...

> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> ---
> 
> Changes in v7: New patch
> 
>  .../ethernet/oki-semi/pch_gbe/pch_gbe_main.c  | 26 ++++++++++---------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> index 9651fa02d4bb..5157cea16773 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -617,8 +617,10 @@ static void pch_gbe_init_stats(struct pch_gbe_adapter *adapter)
>  static int pch_gbe_init_phy(struct pch_gbe_adapter *adapter)
>  {
>  	struct net_device *netdev = adapter->netdev;
> +	struct pch_gbe_hw *hw = &adapter->hw;
>  	u32 addr;
>  	u16 bmcr, stat;
> +	s32 ret_val;
>  
>  	/* Discover phy addr by searching addrs in order {1,0,2,..., 31} */
>  	for (addr = 0; addr < PCH_GBE_PHY_REGS_LEN; addr++) {
> @@ -652,6 +654,16 @@ static int pch_gbe_init_phy(struct pch_gbe_adapter *adapter)
>  	adapter->mii.mdio_read = pch_gbe_mdio_read;
>  	adapter->mii.mdio_write = pch_gbe_mdio_write;
>  	adapter->mii.supports_gmii = mii_check_gmii_support(&adapter->mii);
> +
> +	ret_val = pch_gbe_phy_get_id(hw);
> +	if (ret_val) {
> +		netdev_err(adapter->netdev, "pch_gbe_phy_get_id error\n");
> +		return -EIO;
> +	}
> +	pch_gbe_phy_init_setting(hw);
> +	/* Setup Mac interface option RGMII */
> +	pch_gbe_phy_set_rgmii(hw);
> +
>  	return 0;
>  }
>  
> @@ -721,22 +733,12 @@ void pch_gbe_reset(struct pch_gbe_adapter *adapter)
>  {
>  	struct net_device *netdev = adapter->netdev;
>  	struct pch_gbe_hw *hw = &adapter->hw;
> -	s32 ret_val;
>  
>  	pch_gbe_mac_reset_hw(hw);
>  	/* reprogram multicast address register after reset */
>  	pch_gbe_set_multi(netdev);
>  	/* Setup the receive address. */
>  	pch_gbe_mac_init_rx_addrs(hw, PCH_GBE_MAR_ENTRIES);
> -
> -	ret_val = pch_gbe_phy_get_id(hw);
> -	if (ret_val) {
> -		netdev_err(adapter->netdev, "pch_gbe_phy_get_id error\n");
> -		return;
> -	}
> -	pch_gbe_phy_init_setting(hw);
> -	/* Setup Mac interface option RGMII */
> -	pch_gbe_phy_set_rgmii(hw);
>  }
>  
>  /**

Up to here, everything looks O.K.

> @@ -2577,6 +2579,8 @@ static int pch_gbe_probe(struct pci_dev *pdev,
>  	if (ret)
>  		goto err_free_netdev;
>  
> +	pch_gbe_check_options(adapter);
> +
>  	/* Initialize PHY */
>  	ret = pch_gbe_init_phy(adapter);
>  	if (ret) {
> @@ -2606,8 +2610,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
>  
>  	INIT_WORK(&adapter->reset_task, pch_gbe_reset_task);
>  
> -	pch_gbe_check_options(adapter);
> -
>  	/* initialize the wol settings based on the eeprom settings */
>  	adapter->wake_up_evt = PCH_GBE_WL_INIT_SETTING;
>  	dev_info(&pdev->dev, "MAC address : %pM\n", netdev->dev_addr);

But these two changes seem unrelated. Should they be in a different
patch?

	Andrew

^ permalink raw reply

* Re: [patch net-next 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Cong Wang @ 2018-06-27 17:04 UTC (permalink / raw)
  To: sridhar.samudrala
  Cc: Jiri Pirko, Jakub Kicinski, Linux Kernel Network Developers,
	David Miller, Jamal Hadi Salim, Simon Horman, john.hurley,
	David Ahern, mlxsw
In-Reply-To: <0272f671-802b-30b6-6ca2-2ffc1e205664@intel.com>

On Wed, Jun 27, 2018 at 9:46 AM Samudrala, Sridhar
<sridhar.samudrala@intel.com> wrote:
>
> On 6/27/2018 12:50 AM, Jiri Pirko wrote:
> > if you don't like "tc filter template add dev dummy0 ingress", how
> > about:
> > "tc template add dev dummy0 ingress ..."
> > "tc template add dev dummy0 ingress chain 22 ..."
> > that makes more sense I think.

Better than 'tc filter template', but this doesn't reflect 'template'
is a template of tc filter, it could be an action etc., since it is in the
same position with 'tc action/filter/qdisc'.


>
> Isn't it possible to avoid introducing another keyword 'template',
>
> Can't we just do
>        tc chain add dev dummy0 ingress flower chain_index 0
> to create a chain that takes any types of flower rules with index 0
> and
>       tc chain add dev dummy0 ingress flower chain_index 22
>              dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>       tc chain add dev dummy0 ingress flower chain_index 23
>              dst_ip 192.168.0.0/16
> to create 2 chains 22 and 23 that allow rules with specific fields.

Sounds good too. Since filter chain can be shared by qdiscs,
a 'tc chain' sub-command makes sense, and would probably make
it easier to be shared.

^ permalink raw reply

* Re: [bpf-next PATCH 1/2] samples/bpf: extend xdp_rxq_info to read packet payload
From: Song Liu @ 2018-06-27 17:03 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
	Alexei Starovoitov
In-Reply-To: <20180627132312.0640448b@redhat.com>

On Wed, Jun 27, 2018 at 4:23 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Tue, 26 Jun 2018 16:53:15 -0700
> Song Liu <liu.song.a23@gmail.com> wrote:
>
>> > +static char* options2str(enum cfg_options_flags flag)
>> > +{
>> > +       if (flag == NO_TOUCH)
>> > +               return "no_touch";
>> > +       if (flag & READ_MEM)
>> > +               return "read";
>> > +       fprintf(stderr, "ERR: Unknown config option flags");
>> > +       exit(EXIT_FAIL);
>> > +}
>> > +
>>
>> enum cfg_options_flags is used as a bitmap in other parts of the sample.
>> So this function is a little weird (with more flags added).
>
> Sure, and do I handle this correctly in the next patch.
>
> I'm uncertain what you want me to change?
> Do you want me to drop the enum, and use #define instead?

I think  it is good  as-is for sample code.

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [bpf-next PATCH 2/2] samples/bpf: xdp_rxq_info action XDP_TX must adjust MAC-addrs
From: Song Liu @ 2018-06-27 17:02 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Networking, Daniel Borkmann, Toke Høiland-Jørgensen,
	Alexei Starovoitov
In-Reply-To: <20180627132026.79190a42@redhat.com>

On Wed, Jun 27, 2018 at 4:20 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Tue, 26 Jun 2018 17:09:01 -0700
> Song Liu <liu.song.a23@gmail.com> wrote:
>
>> On Mon, Jun 25, 2018 at 7:27 AM, Jesper Dangaard Brouer
>> <brouer@redhat.com> wrote:
>> > XDP_TX requires also changing the MAC-addrs, else some hardware
>> > may drop the TX packet before reaching the wire.  This was
>> > observed with driver mlx5.
>> >
>> > If xdp_rxq_info select --action XDP_TX the swapmac functionality
>> > is activated.  It is also possible to manually enable via cmdline
>> > option --swapmac.  This is practical if wanting to measure the
>> > overhead of writing/updating payload for other action types.
>> >
>> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>> > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> > ---
> [...]
>> >
>> > diff --git a/samples/bpf/xdp_rxq_info_kern.c b/samples/bpf/xdp_rxq_info_kern.c
>> > index 61af6210df2f..222a83eed1cb 100644
>> > --- a/samples/bpf/xdp_rxq_info_kern.c
>> > +++ b/samples/bpf/xdp_rxq_info_kern.c
>> > @@ -21,6 +21,7 @@ struct config {
>> >  enum cfg_options_flags {
>> >         NO_TOUCH = 0x0U,
>> >         READ_MEM = 0x1U,
>> > +       SWAP_MAC = 0x2U,
>> >  };
> [...]
>> > @@ -98,7 +116,7 @@ int  xdp_prognum0(struct xdp_md *ctx)
>> >                 rxq_rec->issue++;
>> >
>> >         /* Default: Don't touch packet data, only count packets */
>> > -       if (unlikely(config->options & READ_MEM)) {
>> > +       if (unlikely(config->options & (READ_MEM|SWAP_MAC))) {
>> >                 struct ethhdr *eth = data;
>> >
>> >                 if (eth + 1 > data_end)
> [...]
>
>> > diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
>> > index 435485d4f49e..248a7eab9531 100644
> [...]
>> > @@ -119,6 +121,8 @@ static char* options2str(enum cfg_options_flags flag)
>> >  {
>> >         if (flag == NO_TOUCH)
>> >                 return "no_touch";
>> > +       if (flag & SWAP_MAC)
>> > +               return "swapmac";
>> >         if (flag & READ_MEM)
>> >                 return "read";
>>
>> I guess SWAP_MAC also reads the memory, so it "includes" READ_MEM?
>
> True (see _kern side)
>
>> It is OK for now. We may need to refactor this part when adding other
>> flags in the future.
>
> Sure, do remember that this is only a 'sample' program.

Agreed.

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: force cwnd at least 2 in tcp_cwnd_reduction
From: Yuchung Cheng @ 2018-06-27 16:57 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Lawrence Brakmo, Matt Mathis, Netdev, Kernel Team, Blake Matheny,
	Alexei Starovoitov, Eric Dumazet, Wei Wang
In-Reply-To: <CADVnQymQc0Z5q0p=Cs3nJ1D+QrO7Pc78n287XYLDC_y5vBEN+Q@mail.gmail.com>

On Wed, Jun 27, 2018 at 8:24 AM, Neal Cardwell <ncardwell@google.com> wrote:
> On Tue, Jun 26, 2018 at 10:34 PM Lawrence Brakmo <brakmo@fb.com> wrote:
>> The only issue is if it is safe to always use 2 or if it is better to
>> use min(2, snd_ssthresh) (which could still trigger the problem).
>
> Always using 2 SGTM. I don't think we need min(2, snd_ssthresh), as
> that should be the same as just 2, since:
>
> (a) RFCs mandate ssthresh should not be below 2, e.g.
> https://tools.ietf.org/html/rfc5681 page 7:
>
>  ssthresh = max (FlightSize / 2, 2*SMSS)            (4)
>
> (b) The main loss-based CCs used in Linux (CUBIC, Reno, DCTCP) respect
> that constraint, and always have an ssthresh of at least 2.
>
> And if some CC misbehaves and uses a lower ssthresh, then taking
> min(2, snd_ssthresh) will trigger problems, as you note.
>
>> +       tp->snd_cwnd = max((int)tcp_packets_in_flight(tp) + sndcnt, 2);
>
> AFAICT this does seem like it will make the sender behavior more
> aggressive in cases with high loss and/or a very low per-flow
> fair-share.
>
> Old:
>
> o send N packets
> o receive SACKs for last 3 packets
> o fast retransmit packet 1
> o using ACKs, slow-start upward
>
> New:
>
> o send N packets
> o receive SACKs for last 3 packets
> o fast retransmit packets 1 and 2
> o using ACKs, slow-start upward
>
> In the extreme case, if the available fair share is less than 2
> packets, whereas inflight would have oscillated between 1 packet and 2
> packets with the existing code, it now seems like with this commit the
> inflight will now hover at 2. It seems like this would have
> significantly higher losses than we had with the existing code.
I share similar concern. Note that this function is used by most
existing congestion control modules beside DCTCP so I am more cautious
of changing this to address DCTCP issue.

One problem that DCTCP paper notices when cwnd = 1 is still too big
when the bottleneck
is shared by many flows (e.g. incast). It specifically suggest
changing the lower-bound of 2 in the spec to 1. (Section 8.2).
https://www.usenix.org/system/files/conference/nsdi15/nsdi15-paper-judd.pdf

I am curious about the differences you observe in 4.11 and 4.16. I
wasn't aware of any (significant) change in tcp_cwnd_reduction / PRR
algorithm between 4.11 and 4.16. Also the receiver should not delay
ACKs if it has out-of-order packet or receiving CE data packets. This
means the delayed ACK is by tail losses and the last data received
carries no CE mark: seems a less common scenario?

If delayed-ACK is the problem, we probably should fix the receiver to
delay ACK more intelligently, not the sender. weiwan@google.com is
working on it.



>
> This may or may not be OK in practice, but IMHO it is worth mentioning
> and discussing.
>
> neal

^ permalink raw reply

* Re: [patch net-next 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Samudrala, Sridhar @ 2018-06-27 16:46 UTC (permalink / raw)
  To: Jiri Pirko, Jakub Kicinski
  Cc: Linux Netdev List, David Miller, Jamal Hadi Salim, Cong Wang,
	Simon Horman, John Hurley, David Ahern, mlxsw
In-Reply-To: <20180627075017.GA2007@nanopsycho>

On 6/27/2018 12:50 AM, Jiri Pirko wrote:
> Tue, Jun 26, 2018 at 11:18:58PM CEST, jakub.kicinski@netronome.com wrote:
>> On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:
>>> Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicinski@netronome.com wrote:
>>>> On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>>>>> Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicinski@netronome.com wrote:
>>>>>> On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:
>>>>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>>>>
>>>>>>> For the TC clsact offload these days, some of HW drivers need
>>>>>>> to hold a magic ball. The reason is, with the first inserted rule inside
>>>>>>> HW they need to guess what fields will be used for the matching. If
>>>>>>> later on this guess proves to be wrong and user adds a filter with a
>>>>>>> different field to match, there's a problem. Mlxsw resolves it now with
>>>>>>> couple of patterns. Those try to cover as many match fields as possible.
>>>>>>> This aproach is far from optimal, both performance-wise and scale-wise.
>>>>>>> Also, there is a combination of filters that in certain order won't
>>>>>>> succeed.
>>>>>>>
>>>>>>> Most of the time, when user inserts filters in chain, he knows right away
>>>>>>> how the filters are going to look like - what type and option will they
>>>>>>> have. For example, he knows that he will only insert filters of type
>>>>>>> flower matching destination IP address. He can specify a template that
>>>>>>> would cover all the filters in the chain.
>>>>>> Perhaps it's lack of sleep, but this paragraph threw me a little off
>>>>>> the track.  IIUC the goal of this set is to provide a way to inform the
>>>>>> HW about expected matches before any rule is programmed into the HW.
>>>>>> Not before any rule is added to a particular chain.  One can just use
>>>>>> the first rule in the chain to make a guess about the chain, but thanks
>>>>>> to this set user can configure *all* chains before any rules are added.
>>>>> The template is per-chain. User can use template for chain x and
>>>>> not-use it for chain y. Up to him.
>>>> Makes sense.
>>>>
>>>> I can't help but wonder if it'd be better to associate the
>>>> constraints/rules with chains instead of creating a new "template"
>>>> object.  It seems more natural to create a chain with specific
>>>> constraints in place than add and delete template of which there can
>>>> be at most one to a chain...  Perhaps that's more about the user space
>>>> tc command line.  Anyway, not a strong objection, just a thought.
>>> Hmm. I don't think it is good idea. User should see the template in a
>>> "show" command per chain. We would have to have 2 show commands, one to
>>> list the template objects and one to list templates per chains. It makes
>>> things more complicated for no good reason. I think that this simple
>>> chain-lock is easier and serves the purpose.
>> Hm, I think the dump is fine, what I was thinking about was:
>>
>> # tc chain add dev dummy0 ingress chain_index 22 \
>>      ^^^^^
>> 	template proto ip \
>> 	^^^^^^^^
>> 	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
> Okay, I got it. I see 2 issues.
> 1) user might expect to add a chain without the template. But that does
>     not make sense really. Chains are created/deleted implicitly
>     according to refcount.
> 2) there is not chain object like this available to user. Adding it just
>     for template looks odd. Also, the "filter" and "template" are very
>     much alike. They both are added to a chain, they both implicitly
>     create chain if it does not exist, etc.
>
> if you don't like "tc filter template add dev dummy0 ingress", how
> about:
> "tc template add dev dummy0 ingress ..."
> "tc template add dev dummy0 ingress chain 22 ..."
> that makes more sense I think.

Isn't it possible to avoid introducing another keyword 'template',

Can't we just do
       tc chain add dev dummy0 ingress flower chain_index 0
to create a chain that takes any types of flower rules with index 0
and
      tc chain add dev dummy0 ingress flower chain_index 22
             dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
      tc chain add dev dummy0 ingress flower chain_index 23
             dst_ip 192.168.0.0/16
to create 2 chains 22 and 23 that allow rules with specific fields.

To create a chain that takes decap rules like
   filter protocol ip pref 2 flower chain 25 handle 0x2
     dst_mac fe:24:9a:23:4c:5c
     src_mac ce:1d:58:eb:a0:35
     eth_type ipv4
     enc_dst_ip 192.168.100.20
     enc_src_ip 192.168.100.22
     enc_key_id 100
     enc_dst_port 4789

can we create a chain using this format

tc chain add dev dummy0 ingress flower chain_index 25
     dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
     src_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
     enc_dst_ip 192.168.100.0/24
     enc_src_ip 192.168.100.0/24
     eth_type ipv4/arp
     enc_key_id 0/ffff
     enc_dst_port 0/ffff

When adding a filter to a pre-defined chain, does the filter need
to specify all the fields that are included in the chain's template?

>
>
>> instead of:
>>
>> # tc filter template add dev dummy0 ingress \
>>      ^^^^^^^^^^^^^^^
>> 	proto ip chain_index 22 \
>> 	flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
>>
>> And then delete becomes:
>>
>> # tc chain del dev dummy0 ingress chain_index 22
>> Error: The chain is not empty.
>>
>> The fact that template is very much like a filter is sort of an
>> implementation detail, from user perspective it may be more intuitive
>> to model template as an attribute of the chain, not a filter object
>> added to a chain.
>>
>> But I could well be the only person who feels that way :)

^ permalink raw reply

* RE: [RFC bpf-next 0/6] XDP RX device meta data acceleration (WIP)
From: Parikh, Neerav @ 2018-06-27 16:42 UTC (permalink / raw)
  To: Saeed Mahameed, Jesper Dangaard Brouer, Alexei Starovoitov,
	Daniel Borkmann
  Cc: pjwaskiewicz@gmail.com, ttoukan.linux@gmail.com, Tariq Toukan,
	Duyck, Alexander H, Waskiewicz Jr, Peter, Opher Reviv,
	Rony Efraim, netdev@vger.kernel.org, Saeed Mahameed
In-Reply-To: <20180627024615.17856-1-saeedm@mellanox.com>

Thanks Saeed for starting this thread  :)
My comments inline. 

> -----Original Message-----
> From: Saeed Mahameed [mailto:saeedm@dev.mellanox.co.il]
> Sent: Tuesday, June 26, 2018 7:46 PM
> To: Jesper Dangaard Brouer <brouer@redhat.com>; Alexei Starovoitov
> <alexei.starovoitov@gmail.com>; Daniel Borkmann
> <borkmann@iogearbox.net>
> Cc: Parikh, Neerav <neerav.parikh@intel.com>; pjwaskiewicz@gmail.com;
> ttoukan.linux@gmail.com; Tariq Toukan <tariqt@mellanox.com>; Duyck,
> Alexander H <alexander.h.duyck@intel.com>; Waskiewicz Jr, Peter
> <peter.waskiewicz.jr@intel.com>; Opher Reviv <opher@mellanox.com>; Rony
> Efraim <ronye@mellanox.com>; netdev@vger.kernel.org; Saeed Mahameed
> <saeedm@mellanox.com>
> Subject: [RFC bpf-next 0/6] XDP RX device meta data acceleration (WIP)
> 
> Hello,
> 
> Although it is far from being close to completion, this series provides
> the means and infrastructure to enable device drivers to share packets
> meta data information and accelerations with XDP programs,
> such as hash, stripped vlan, checksum, flow mark, packet header types,
> etc ..
> 
> The series is still work in progress and sending it now as RFC in order
> to get early feedback and to discuss the design, structures and UAPI.
> 
> For now the general idea is to help XDP programs accelerate, and provide
> them with meta data that already available from the HW or device driver
> to save cpu cycles on packet headers and data processing and decision
> making, aside of that we want to avoid having a fixed size meta data
> structure that wastes a lot of buffer space for stuff that the xdp program
> might not need, like the current "elephant" SKB fields, kidding :) ..
> 
> So my idea here is to provide a dynamic mechanism to allow XDP programs
> on xdp load to request only the specific meta data they need, and
> the device driver or netdevice will provide them in a predefined order
> in the xdp_buff/xdp_md data meta section.
>
> And here is how it is done and what i would like to discuss:
> 
> 1. In the first patch: added the infrastructure to request predefined meta data
> flags on xdp load indicating that the XDP program is going to need them.
> 
> 1.1) In this patch I am using the current u32 bit IFLA_XDP_FLAGS,
> TODO: this needs to be improved in order to allow more meta data flags,
> maybe use a new dedicated flags ?
> 
> 1.2) Device driver that want to support xdp meta data should implement
> new XDP command XDP_QUERY_META_FLAGS and report the meta data flags
> they
> can support.
> 
> 1.3) the kernel will cross check the requested flags with the device's
> supported flags and will fail the xdp load in case of discrepancy.
> 
> Question : do we want this ? or is it better to return to the program
> the actual supported flags and let it decide ?
> 
>
The work we are doing in this direction does not assume any specific flags but
instead the XDP program requests for certain "meta data" that it needs and
the driver (or HW) if can provide that then allow loading of that program.
If we put the flags and capabilities in kernel then it will depend on the control
program loading the program to pass on that information. If the XDP program
has that built-in via say ELF "section" (similar to maps) then the program
can be loaded independently and knows what kind of meta data it wants
and receives. 
If the meta data is not supported by the device (driver or software mode)
then that would perhaps fail the program load.
 
> 2. This is the interesting part: in the 2nd patch Added the meta data
> set/get infrastructure to allow device drivers populate them into xdp_buff
> data meta in a well defined and structured format and let the xdp program
> read them according to the predefined format/structure, the idea here is
> that the xdp program and the device driver will share a static information
> offsets array that will define where each meta data will sit inside
> xdp_buff data meta, such structure will be decided once on xdp load.
> 
> Enters struct xdp_md_info and xdp_md_info_arr:
> 
> struct xdp_md_info {
>        __u16 present:1;
>        __u16 offset:15; /* offset from data_meta in xdp_md buffer */
> };
We were trying to not define a generic structure if possible and were working
towards a model similar to current usage where the XDP program produces
a meta data that is consumed by the eBPF program on the TC classifier without
hard coding any structure. It's purely a producer-consumer model with the
kernel helping transfer that data from one end to another instead of providing
a structure. 
So, the NIC produces the meta data requested by the XDP program v/s producing
some meta data then that gets translated in drivers into a generic structure.


> 
> /* XDP meta data offsets info array
>  * present bit describes if a meta data is or will be present in xdp_md buff
>  * offset describes where a meta data is or should be placed in xdp_md buff
>  *
>  * Kernel builds this array using xdp_md_info_build helper on demand.
>  * User space builds it statically in the xdp program.
>  */
> typedef struct xdp_md_info xdp_md_info_arr[XDP_DATA_META_MAX];
> 
> Offsets in xdp_md_info_arr are always in ascending order and only for
> requested meta data:
> example : for XDP prgram that requested the following flags:
> flags = XDP_FLAGS_META_HASH | XDP_FLAGS_META_VLAN;
> 
> the offsets array will be:
> 
> xdp_md_info_arr mdi = {
>         [XDP_DATA_META_HASH] = {.offset = 0, .present = 1},
>         [XDP_DATA_META_MARK] = {.offset = 0, .present = 0},
>         [XDP_DATA_META_VLAN] = {.offset = sizeof(struct xdp_md_hash), .present
> = 1},
>         [XDP_DATA_META_CSUM] = {.offset = 0, .present = 0},
> }
> 
> For this example: hash fields will always appear first then the vlan for every
> xdp_md.
> 
> Once requested to provide xdp meta data, device driver will use a pre-built
> xdp_md_info_arr which was built via xdp_md_info_build on xdp setup,
> xdp_md_info_arr will tell the driver what is the offset of each meta data.
> The user space XDP program will use a similar xdp_md_info_arr to
> statically know what is the offset of each meta data.
> 
> *For future meta data they will be added to the end of the array with
> higher flags value.
> 
> This patch also provides helper functions for the device drivers to store
> meta data into xdp_buff, and helper function for XDP programs to fetch
> specific xdp meta data from xdp_md buffer.
> 
> Question: currently the XDP program is responsible to build the static
> meta data offsets array "xdp_md_info_arr" and the kernel will build it
> for the netdevice, if we are going to choose this direction, should we
> somehow share the same xdp_md_info_arr built by the kernel with the xdp
> program ?
> 
> 3. The last mlx5e patch is the actual show case of how the device driver
> will add the support for xdp meta data, it is pretty simple and straight
> forward ! The first two mlx5e patches are just small refactoring to make
> the xdp_md_info_arr and packet completion information available in the rx
> xdp handlers data path.
> 
> 4. Just added a small example to demonstrate how XDP program can request
> meta data on xdp load and how it can read them on the critical path.
> of course more examples are needed and some performance numbers.
> Exciting use cases such as:
>   - using flow mark to allow fast white/black listing lookups.
>   - flow mark to accelerate DDos prevention.
>   - Generic Data path: Use the meta data from the xdp_buff to build SKBs
> !(Jesper's Idea)
>   - using hash type to know the packet headers and encapsulation without
>     touching the packet data at all.
>   - using packet hash to do RPS and XPS like cpu redirecting.
>   - and many more.
> 
> 
> More ideas:
> 
> From Jesper:
> =========================
> Give XDP more rich information about the hash,
> By reducing the 'ht' value to the PKT_HASH_TYPE's we are loosing information.
> 
> I happen to know your hardware well; CQE_RSS_HTYPE_IP tell us if this
> is IPv4 or IPv6.  And CQE_RSS_HTYPE_L4 tell us if this is TCP, UDP or
> IPSEC. (And you have another bit telling of this is IPv6 with extension
> headers).
> 
Yes the commodity NICs have lot of rich information about packet parsing and
protocol information. It would be worth extending this for the XDP programs so
that they can take advantage of as much work done by the NIC already v/s
redoing all that in the XDP program. 
So, basically that will allow XDP programs to put more focus on the 
"business logic" of whatever it determines to do with the packet. 


> If we don't want to invent our own xdp_hash_types, we can simply adopt
> the RSS Hashing Types defined by Microsoft:
>  https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-
> hashing-types
> 
> An interesting part of the RSS standard, is that the hash type can help
> identify if this is a fragment. (XDP could use this info to avoid
> touching payload and react, e.g. drop fragments, or redirect all
> fragments to another CPU, or skip parsing in XDP and defer to network
> stack via XDP_PASS).
> 
Yes as well it would be interesting if there is use if XDP program would like to
monitor any packet errors that were captured by the NIC to monitor. 
While traditionally the driver may decide to drop/consume such packets but
it would be a good use-case for debugging.

> By using the RSS standard, we do e.g. loose the ability to say this is
> IPSEC traffic, even-though your HW supports this.
> 
> I have tried to implemented my own (non-dynamic) XDP RX-types UAPI here:
>  https://marc.info/?l=linux-netdev&m=149512213531769
>  https://marc.info/?l=linux-netdev&m=149512213631774
> =========================
> 
> Thanks,
> Saeed.
> 
> Saeed Mahameed (6):
>   net: xdp: Add support for meta data flags requests
>   net: xdp: RX meta data infrastructure
>   net/mlx5e: Store xdp flags and meta data info
>   net/mlx5e: Pass CQE to RX handlers
>   net/mlx5e: Add XDP RX meta data support
>   samples/bpf: Add meta data hash example to xdp_redirect_cpu
> 
>  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  19 ++-
>  .../net/ethernet/mellanox/mlx5/core/en_main.c |  58 +++++----
>  .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  47 ++++++--
>  include/linux/netdevice.h                     |  10 ++
>  include/net/xdp.h                             |   6 +
>  include/uapi/linux/bpf.h                      | 114 ++++++++++++++++++
>  include/uapi/linux/if_link.h                  |  20 ++-
>  net/core/dev.c                                |  41 +++++++
>  samples/bpf/xdp_redirect_cpu_kern.c           |  67 ++++++++++
>  samples/bpf/xdp_redirect_cpu_user.c           |   7 ++
>  10 files changed, 354 insertions(+), 35 deletions(-)
> 
> --
> 2.17.0

^ permalink raw reply

* Re: [RFC PATCH v2 net-next 06/12] net: core: propagate SKB lists through packet_type lookup
From: Edward Cree @ 2018-06-27 16:34 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: linux-net-drivers, Network Development, David Miller
In-Reply-To: <CAF=yD-KSJRO9SFE+bNNAWXihTi6VvD3pvUC36OJzSv9vNXS6vg@mail.gmail.com>

On 27/06/18 17:00, Willem de Bruijn wrote:
> On Wed, Jun 27, 2018 at 10:49 AM Edward Cree <ecree@solarflare.com> wrote:
>> On 27/06/18 15:36, Willem de Bruijn wrote:
>>> Also, this function does more than just process network taps.
>> This is true, but naming things is hard, and I couldn't think of either a
>>  better new name for this function or a name that could fit in between
>>  __netif_receive_skb() and __netif_receive_skb_core() for the new function
>>  in my patch named __netif_receive_skb_core().  Any suggestions?
> ____netif_receive_skb_core? Not that four underscores is particularly
> readable. Perhaps __netif_receive_skb_core_inner. It's indeed tricky (and
> not the most important, I didn't mean to bikeshed).
I've gone with __netif_receive_skb_one_core() (by contrast to ..._list_core())
 for the outer function.  And I don't mind when people shed bikes :)

> Come to think of it, from your fast path assumptions, we could perhaps wrap
> ptype_all and rx_handler logic in a static_branch similar to tc and netfilter
> (and sk_memalloc_socks). Remaining branches like skip_classify, pfmemalloc
> and deliver_exact can also not be reached if all these are off, so this entire
> section can be skipped. Then it could become __netif_receive_skb_slow,
> taken only on the static branch or for vlan packets.  I do not suggest it as
> part of this patchset. it would be a pretty complex change on its own.

That is an interesting idea, but agreed that it'd be quite complex.

^ permalink raw reply

* Re: [net] bpfilter: include bpfilter_umh in assembly instead of using objcopy
From: Guenter Roeck @ 2018-06-27 16:18 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S . Miller, daniel, torvalds, mcroce, yamada.masahiro,
	netdev, kernel-team
In-Reply-To: <20180627031348.285964-1-ast@kernel.org>

On Tue, Jun 26, 2018 at 08:13:48PM -0700, Alexei Starovoitov wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>
> 
> What we want here is to embed a user-space program into the kernel.
> Instead of the complex ELF magic, let's simply wrap it in the assembly
> with the '.incbin' directive.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> I think this patch should 'fix' bpfilter build issue on all archs.
> cflags for cross CC may still be incorrect and embedded blob
> may fail to execute via fork_usermode_blob()
> (like in case of 'make ARCH=i386 net/bpfilter/' CC will build and link 64-bit
> binary that will be included into bpfilter.o or vmlinux and that binary
> will fail to run on 32-bit kernel),
> but that is separate issue that will be addressed in net-next time frame.
> Long term we've discussed to switch to something like klibc and keep it
> as part of the kernel to avoid relying on glibc and cc-can-link.sh.
> 

At the very least it fixes the build issue when building i386 images
on x86_64 systems.

(Build-)Tested-by: Guenter Roeck <linux@roeck-us.net>

>  net/bpfilter/Makefile            | 17 ++---------------
>  net/bpfilter/bpfilter_kern.c     | 11 +++++------
>  net/bpfilter/bpfilter_umh_blob.S |  7 +++++++
>  3 files changed, 14 insertions(+), 21 deletions(-)
>  create mode 100644 net/bpfilter/bpfilter_umh_blob.S
> 
> diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
> index 051dc18b8ccb..39c6980b5d99 100644
> --- a/net/bpfilter/Makefile
> +++ b/net/bpfilter/Makefile
> @@ -15,20 +15,7 @@ ifeq ($(CONFIG_BPFILTER_UMH), y)
>  HOSTLDFLAGS += -static
>  endif
>  
> -# a bit of elf magic to convert bpfilter_umh binary into a binary blob
> -# inside bpfilter_umh.o elf file referenced by
> -# _binary_net_bpfilter_bpfilter_umh_start symbol
> -# which bpfilter_kern.c passes further into umh blob loader at run-time
> -quiet_cmd_copy_umh = GEN $@
> -      cmd_copy_umh = echo ':' > $(obj)/.bpfilter_umh.o.cmd; \
> -      $(OBJCOPY) -I binary \
> -          `LC_ALL=C $(OBJDUMP) -f net/bpfilter/bpfilter_umh \
> -          |awk -F' |,' '/file format/{print "-O",$$NF} \
> -          /^architecture:/{print "-B",$$2}'` \
> -      --rename-section .data=.init.rodata $< $@
> -
> -$(obj)/bpfilter_umh.o: $(obj)/bpfilter_umh
> -	$(call cmd,copy_umh)
> +$(obj)/bpfilter_umh_blob.o: $(obj)/bpfilter_umh
>  
>  obj-$(CONFIG_BPFILTER_UMH) += bpfilter.o
> -bpfilter-objs += bpfilter_kern.o bpfilter_umh.o
> +bpfilter-objs += bpfilter_kern.o bpfilter_umh_blob.o
> diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
> index 09522573f611..f0fc182d3db7 100644
> --- a/net/bpfilter/bpfilter_kern.c
> +++ b/net/bpfilter/bpfilter_kern.c
> @@ -10,11 +10,8 @@
>  #include <linux/file.h>
>  #include "msgfmt.h"
>  
> -#define UMH_start _binary_net_bpfilter_bpfilter_umh_start
> -#define UMH_end _binary_net_bpfilter_bpfilter_umh_end
> -
> -extern char UMH_start;
> -extern char UMH_end;
> +extern char bpfilter_umh_start;
> +extern char bpfilter_umh_end;
>  
>  static struct umh_info info;
>  /* since ip_getsockopt() can run in parallel, serialize access to umh */
> @@ -93,7 +90,9 @@ static int __init load_umh(void)
>  	int err;
>  
>  	/* fork usermode process */
> -	err = fork_usermode_blob(&UMH_start, &UMH_end - &UMH_start, &info);
> +	err = fork_usermode_blob(&bpfilter_umh_start,
> +				 &bpfilter_umh_end - &bpfilter_umh_start,
> +				 &info);
>  	if (err)
>  		return err;
>  	pr_info("Loaded bpfilter_umh pid %d\n", info.pid);
> diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S
> new file mode 100644
> index 000000000000..40311d10d2f2
> --- /dev/null
> +++ b/net/bpfilter/bpfilter_umh_blob.S
> @@ -0,0 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +	.section .init.rodata, "a"
> +	.global bpfilter_umh_start
> +bpfilter_umh_start:
> +	.incbin "net/bpfilter/bpfilter_umh"
> +	.global bpfilter_umh_end
> +bpfilter_umh_end:

^ permalink raw reply

* Re: [Patch net-next v3 1/3] net: introduce helper dev_change_tx_queue_len()
From: Eric Dumazet @ 2018-06-27 16:14 UTC (permalink / raw)
  To: Cong Wang, netdev; +Cc: john.fastabend
In-Reply-To: <20180126022624.20442-2-xiyou.wangcong@gmail.com>



On 01/25/2018 06:26 PM, Cong Wang wrote:
> This patch promotes the local change_tx_queue_len() to a core
> helper function, dev_change_tx_queue_len(), so that rtnetlink
> and net-sysfs could share the code. This also prepares for the
> following patch.
> 
> Note, the -EFAULT in the original code doesn't make sense,
> we should propagate the errno from notifiers.
> 
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  include/linux/netdevice.h |  1 +
>  net/core/dev.c            | 28 ++++++++++++++++++++++++++++
>  net/core/net-sysfs.c      | 25 +------------------------
>  net/core/rtnetlink.c      | 18 +++++-------------
>  4 files changed, 35 insertions(+), 37 deletions(-)
> 

Hi Cong

What about using dev_change_tx_queue_len() helper from SIOCSIFTXQLEN path in
net/core/dev_ioctl.c ?

This would make sure we call dev_qdisc_change_tx_queue_len() in this case.

Thanks !

^ permalink raw reply

* Re: [2/4] libertas_tf: use irqsave() in USB's complete callback
From: Kalle Valo @ 2018-06-27 16:13 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-usb, tglx, David S. Miller, linux-wireless,
	Sebastian Andrzej Siewior
In-Reply-To: <20180620193648.25136-3-bigeasy@linutronix.de>

Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> The USB completion callback does not disable interrupts while acquiring
> the lock. We want to remove the local_irq_disable() invocation from
> __usb_hcd_giveback_urb() and therefore it is required for the callback
> handler to disable the interrupts while acquiring the lock.
> The callback may be invoked either in IRQ or BH context depending on the
> USB host controller.
> Use the _irqsave() variant of the locking primitives.
> 
> I am removing the
> 	BUG_ON(!in_interrupt());
> 
> check because it serves no purpose. Running the completion callback in
> BH context makes in_interrupt() still return true but the interrupts
> could be enabled. The important part is that ->driver_lock is acquired
> with disabled interrupts which is the case now.
> 
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-wireless@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

3 patches applied to wireless-drivers-next.git, thanks.

fc75122fabb5 libertas_tf: use irqsave() in USB's complete callback
a3128feef6d5 libertas: use irqsave() in USB's complete callback
81454b8405f2 zd1211rw: use irqsave() in USB's complete callback

-- 
https://patchwork.kernel.org/patch/10478609/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* Re: [2/4] zd1211rw: stop using deprecated get_seconds()
From: Kalle Valo @ 2018-06-27 16:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arnd Bergmann, y2038, netdev, linux-wireless, linux-kernel,
	Ulrich Kunitz, Daniel Drake, David S. Miller
In-Reply-To: <20180618151142.1214422-2-arnd@arndb.de>

Arnd Bergmann <arnd@arndb.de> wrote:

> The get_seconds() function is deprecated because of the y2038 overflow.
> In zd1211rw we don't even care about the absolute value, so this is
> not a problem, but it's equally trivial to change to the non-deprecated
> ktime_get_seconds().
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

2 patches applied to wireless-drivers-next.git, thanks.

71e140b57151 zd1211rw: stop using deprecated get_seconds()
3cade2f3d98a ipw2x00: track time using boottime

-- 
https://patchwork.kernel.org/patch/10471961/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply

* Re: [2/4] zd1211rw: stop using deprecated get_seconds()
From: Kalle Valo @ 2018-06-27 16:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Daniel Drake, Ulrich Kunitz, David S. Miller, y2038,
	Arnd Bergmann, linux-wireless, netdev, linux-kernel
In-Reply-To: <20180618151142.1214422-2-arnd@arndb.de>

Arnd Bergmann <arnd@arndb.de> wrote:

> The get_seconds() function is deprecated because of the y2038 overflow.
> In zd1211rw we don't even care about the absolute value, so this is
> not a problem, but it's equally trivial to change to the non-deprecated
> ktime_get_seconds().
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

2 patches applied to wireless-drivers-next.git, thanks.

71e140b57151 zd1211rw: stop using deprecated get_seconds()
3cade2f3d98a ipw2x00: track time using boottime

-- 
https://patchwork.kernel.org/patch/10471961/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ 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