* 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
* [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
* [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 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 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
* 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 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
* [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 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 6/6] net sched actions: avoid bitwise operation on signed value in pedit
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>
Since char can be unsigned or signed, and bitwise operators may have
implementation-dependent results when performed on signed operands,
declare 'u8 *' operand instead.
Suggested-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Roman Mashak <mrv@mojatatu.com>
---
net/sched/act_pedit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index caa6927a992c..ab151346d3d4 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -311,7 +311,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
}
if (tkey->offmask) {
- char *d, _d;
+ u8 *d, _d;
if (!offset_valid(skb, hoffset + tkey->at)) {
pr_info("tc action pedit 'at' offset %d out of bounds\n",
--
2.7.4
^ permalink raw reply related
* Re: [PATCH rdma-next 08/12] overflow.h: Add arithmetic shift helper
From: Leon Romanovsky @ 2018-06-27 17:39 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Jason Gunthorpe, Doug Ledford, Kees Cook, RDMA mailing list,
Hadar Hen Zion, Matan Barak, Michael J Ruhl, Noa Osherovich,
Raed Salem, Yishai Hadas, Saeed Mahameed, linux-netdev,
linux-kernel
In-Reply-To: <CAKwiHFgchr+6HYOZ4e4e1vzL9cFabe6eonNNM8NTWZypazcuKA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 939 bytes --]
On Wed, Jun 27, 2018 at 11:36:03AM +0200, Rasmus Villemoes wrote:
> On 26 June 2018 at 19:54, Jason Gunthorpe <jgg@mellanox.com> wrote:
>
> > On Tue, Jun 26, 2018 at 10:07:07AM +0200, Rasmus Villemoes wrote:
> > > On 25 June 2018 at 19:11, Jason Gunthorpe <[1]jgg@mellanox.com>
> > wrote:
> > >
> >
> > When thinking about signed cases.. The explicit u64 cast, and
> > implict promotion to typeof(d), produce something counter intuitive,
> > eg:
> >
> > (u64)(s32)-1 == 0xffffffffffffffff
> >
> > Which would result in a shift oucome that is not what anyone would
> > expect, IMHO... So the first version isn't what I'd expect either..
> >
>
> Wouldn't check_shift_overflow(-1, 4, &someint) just put -16 in someint and
> report no overflow? That's what I'd expect, if negative values are to be
> supported at all.
Most if not all the times we don't do shifts on negative values, so I
don't think that we should support them.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Official email
From: F Betrisey @ 2018-06-27 14:17 UTC (permalink / raw)
I’m counsel to late Henry who died in an auto crash with his immediate
family in February 2014, and left an estate of $18 million dollars. You
are entitled to his estate, Henry share same last name with you. Please
contact me, thank you.
^ permalink raw reply
* Re: [Patch net-next v3 1/3] net: introduce helper dev_change_tx_queue_len()
From: Cong Wang @ 2018-06-27 17:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Linux Kernel Network Developers, John Fastabend
In-Reply-To: <e2d5e8ea-0096-1c2e-b7d4-76c1172900c3@gmail.com>
On Wed, Jun 27, 2018 at 9:14 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> 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.
>
Good catch! Will send a patch for net-next.
Thanks.
^ permalink raw reply
* Re: [PATCH v7 09/11] net: pch_gbe: Convert to mdiobus and phylib
From: Andrew Lunn @ 2018-06-27 17:51 UTC (permalink / raw)
To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-10-paul.burton@mips.com>
> @@ -5,7 +5,8 @@
> config PCH_GBE
> tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
> depends on PCI && (X86_32 || COMPILE_TEST)
> - select MII
> + select PHYLIB
> + imply AT803X_PHY if X86_32
> select PTP_1588_CLOCK_PCH
> select NET_PTP_CLASSIFY
That is unusual. I don't think any other MAC driver does this.
If the AT803X driver is not available, it will fall back to the
generic PHY driver. That means RGMII delays will not get set
correctly, no interrupts, no wol, and no workaround for the 8030.
Are any of these relevant to your board?
> @@ -197,16 +151,14 @@ static void pch_gbe_get_regs(struct net_device *netdev,
> struct pch_gbe_hw *hw = &adapter->hw;
> struct pci_dev *pdev = adapter->pdev;
> u32 *regs_buff = p;
> - u16 i, tmp;
> + u16 i;
>
> regs->version = 0x1000000 | (__u32)pdev->revision << 16 | pdev->device;
> for (i = 0; i < PCH_GBE_MAC_REGS_LEN; i++)
> *regs_buff++ = ioread32(&hw->reg->INT_ST + i);
> /* PHY register */
> - for (i = 0; i < PCH_GBE_PHY_REGS_LEN; i++) {
> - pch_gbe_phy_read_reg_miic(&adapter->hw, i, &tmp);
> - *regs_buff++ = tmp;
> - }
> + for (i = 0; i < PCH_GBE_PHY_REGS_LEN; i++)
> + *regs_buff++ = phy_read(adapter->phydev, i);
> }
In general, that is not safe. Some PHYs have pages, and you have no
idea what page is currently selected. If you don't need it, i would
drop this. There are other ways to get access to phy registers, like
miitool, which should do a better job.
Andrew
^ permalink raw reply
* Re: [PATCH v7 10/11] ptp: pch: Allow build on MIPS platforms
From: Andrew Lunn @ 2018-06-27 17:53 UTC (permalink / raw)
To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-11-paul.burton@mips.com>
On Tue, Jun 26, 2018 at 05:06:11PM -0700, Paul Burton wrote:
> Allow the ptp_pch driver to be built on MIPS platforms in preparation
> for use on the MIPS Boston board.
>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Acked-by: Richard Cochran <richardcochran@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> ---
>
> Changes in v7: None
>
> drivers/ptp/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index d137c480db46..fd5f2c6c18ba 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -90,7 +90,7 @@ config DP83640_PHY
>
> config PTP_1588_CLOCK_PCH
> tristate "Intel PCH EG20T as PTP clock"
> - depends on X86_32 || COMPILE_TEST
> + depends on X86_32 || MIPS || COMPILE_TEST
Hi Paul
Is there anything in the code which stops it working on S390? ARM?
X86_64? Can we just remove this depends?
Andrew
^ permalink raw reply
* Re: [PATCH v7 06/11] net: pch_gbe: Only enable MAC when PHY link is active
From: Paul Burton @ 2018-06-27 17:54 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, David S . Miller
In-Reply-To: <20180627173014.GD885@lunn.ch>
Hi Andrew,
On Wed, Jun 27, 2018 at 07:30:14PM +0200, Andrew Lunn wrote:
> 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?
pch_gbe_watchdog() polls for the link coming up or going down, so that's
where we find out that the link is up.
> And what happens on link down? Does the MAC need shutting down? I
> don't see such code here.
Well, depending upon the PHY the RX clock might stop which will prevent
parts of the MAC from functioning properly. Exactly which parts I don't
really know - the EG20T documentation is vague & unclear. I do know
that:
- We won't receive packets any more, of course. This should be fine
without any extra handling because we just won't see any futher DMA
complete interrupts (or the associated bit set when polling).
- A MAC reset won't complete - ie. the pch_gbe_wait_clr_bit() in
pch_gbe_reset()/pch_gbe_reset_hw() will time out. This I think
should be OK because after this patch we won't generally reset the
MAC when the link is down anyway, except perhaps the PCI error_state
case in pch_gbe_down(). I'm not sure what the reset there is for...
- Masking or unmasking MAC address registers won't complete - ie. the
pch_gbe_wait_clr_bit() in pch_gbe_mac_mar_set() or
pch_gbe_set_multi() will time out. This is again when the link is
already known to be up, although there is a case in
__pch_gbe_suspend() which is setting up WoL that I'm not so sure
about...
Thanks,
Paul
^ permalink raw reply
* Re: [PATCH v7 06/11] net: pch_gbe: Only enable MAC when PHY link is active
From: Florian Fainelli @ 2018-06-27 17:54 UTC (permalink / raw)
To: Paul Burton, netdev; +Cc: David S . Miller, Andrew Lunn
In-Reply-To: <20180627000612.27263-7-paul.burton@mips.com>
On 06/26/2018 05:06 PM, 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.
What we ended up doing in the bcmgenet driver is loop back the RX and TX
clocks such that we always have a clock that we can use to perform any
MAC operation, including reset.
Is this an option here? You might also want to split the allocation from
the actual initialization if this is not done already.
>
> 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 | 44 +++++++++----------
> 1 file changed, 22 insertions(+), 22 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 eb290c1edce0..721ce29b6467 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
> @@ -1837,7 +1837,6 @@ static int pch_gbe_request_irq(struct pch_gbe_adapter *adapter)
> int pch_gbe_up(struct pch_gbe_adapter *adapter)
> {
> struct net_device *netdev = adapter->netdev;
> - struct pch_gbe_tx_ring *tx_ring = adapter->tx_ring;
> struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
> int err = -EINVAL;
>
> @@ -1847,14 +1846,6 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
> goto out;
> }
>
> - /* hardware has been reset, we need to reload some things */
> - pch_gbe_set_multi(netdev);
> -
> - pch_gbe_setup_tctl(adapter);
> - pch_gbe_configure_tx(adapter);
> - pch_gbe_setup_rctl(adapter);
> - pch_gbe_configure_rx(adapter);
> -
> err = pch_gbe_request_irq(adapter);
> if (err) {
> netdev_err(netdev,
> @@ -1867,18 +1858,9 @@ int pch_gbe_up(struct pch_gbe_adapter *adapter)
> "Error: can't bring device up - alloc rx buffers pool failed\n");
> goto freeirq;
> }
> - pch_gbe_alloc_tx_buffers(adapter, tx_ring);
> - pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
> adapter->tx_queue_len = netdev->tx_queue_len;
> - pch_gbe_enable_dma_rx(&adapter->hw);
> - pch_gbe_enable_mac_rx(&adapter->hw);
>
> mod_timer(&adapter->watchdog_timer, jiffies);
> -
> - napi_enable(&adapter->napi);
> - pch_gbe_irq_enable(adapter);
> - netif_start_queue(adapter->netdev);
> -
> return 0;
>
> freeirq:
> @@ -1930,6 +1912,8 @@ static void pch_gbe_watchdog(struct timer_list *t)
> {
> struct pch_gbe_adapter *adapter = from_timer(adapter, t,
> watchdog_timer);
> + struct pch_gbe_rx_ring *rx_ring = adapter->rx_ring;
> + struct pch_gbe_tx_ring *tx_ring = adapter->tx_ring;
> struct net_device *netdev = adapter->netdev;
> struct pch_gbe_hw *hw = &adapter->hw;
>
> @@ -1950,12 +1934,32 @@ static void pch_gbe_watchdog(struct timer_list *t)
> }
> hw->mac.link_speed = ethtool_cmd_speed(&cmd);
> hw->mac.link_duplex = cmd.duplex;
> +
> + pch_gbe_reset(adapter);
> +
> /* Set the RGMII control. */
> pch_gbe_set_rgmii_ctrl(adapter, hw->mac.link_speed,
> hw->mac.link_duplex);
> /* Set the communication mode */
> pch_gbe_set_mode(adapter, hw->mac.link_speed,
> hw->mac.link_duplex);
> +
> + pch_gbe_set_multi(netdev);
> + pch_gbe_setup_tctl(adapter);
> + pch_gbe_configure_tx(adapter);
> + pch_gbe_setup_rctl(adapter);
> + pch_gbe_configure_rx(adapter);
> +
> + pch_gbe_alloc_tx_buffers(adapter, tx_ring);
> + pch_gbe_alloc_rx_buffers(adapter, rx_ring, rx_ring->count);
> +
> + pch_gbe_enable_dma_rx(&adapter->hw);
> + pch_gbe_enable_mac_rx(&adapter->hw);
> +
> + napi_enable(&adapter->napi);
> + pch_gbe_irq_enable(adapter);
> + netif_start_queue(adapter->netdev);
> +
> netdev_dbg(netdev,
> "Link is Up %d Mbps %s-Duplex\n",
> hw->mac.link_speed,
> @@ -2568,7 +2572,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> (ETH_HLEN + ETH_FCS_LEN);
>
> pch_gbe_mac_load_mac_addr(&adapter->hw);
> - pch_gbe_mac_reset_hw(&adapter->hw);
>
> /* setup the private structure */
> ret = pch_gbe_sw_init(adapter);
> @@ -2610,9 +2613,6 @@ static int pch_gbe_probe(struct pci_dev *pdev,
> adapter->wake_up_evt = PCH_GBE_WL_INIT_SETTING;
> dev_info(&pdev->dev, "MAC address : %pM\n", netdev->dev_addr);
>
> - /* reset the hardware with the new settings */
> - pch_gbe_reset(adapter);
> -
> ret = register_netdev(netdev);
> if (ret)
> goto err_free_adapter;
>
--
Florian
^ permalink raw reply
* Re: [PATCH v7 11/11] net: pch_gbe: Allow build on MIPS platforms
From: Andrew Lunn @ 2018-06-27 17:54 UTC (permalink / raw)
To: Paul Burton; +Cc: netdev, David S . Miller
In-Reply-To: <20180627000612.27263-12-paul.burton@mips.com>
On Tue, Jun 26, 2018 at 05:06:12PM -0700, Paul Burton wrote:
> Allow the pch_gbe driver to be built on MIPS platforms, allowing its use
> on the MIPS Boston development board.
>
> 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: None
>
> drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
> index 5276f4ff3b63..8e3630b9a9d1 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/Kconfig
> @@ -4,7 +4,7 @@
>
> config PCH_GBE
> tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
> - depends on PCI && (X86_32 || COMPILE_TEST)
> + depends on PCI && (X86_32 || MIPS || COMPILE_TEST)
Same question here as for the PTP driver.
Andrew
^ permalink raw reply
* Re: [RFC bpf-next 2/6] net: xdp: RX meta data infrastructure
From: Saeed Mahameed @ 2018-06-27 17:55 UTC (permalink / raw)
To: saeedm@dev.mellanox.co.il, brouer@redhat.com
Cc: alexander.h.duyck@intel.com, peter.waskiewicz.jr@intel.com,
Rony Efraim, borkmann@iogearbox.net, Tariq Toukan,
neerav.parikh@intel.com, Opher Reviv,
alexei.starovoitov@gmail.com, pjwaskiewicz@gmail.com,
netdev@vger.kernel.org, ttoukan.linux@gmail.com
In-Reply-To: <20180627161517.31f1f7af@redhat.com>
On Wed, 2018-06-27 at 16:15 +0200, Jesper Dangaard Brouer wrote:
> On Tue, 26 Jun 2018 19:46:11 -0700
> Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
>
> > diff --git a/include/net/xdp.h b/include/net/xdp.h
> > index 2deea7166a34..afe302613ae1 100644
> > --- a/include/net/xdp.h
> > +++ b/include/net/xdp.h
> > @@ -138,6 +138,12 @@ xdp_set_data_meta_invalid(struct xdp_buff
> > *xdp)
> > xdp->data_meta = xdp->data + 1;
> > }
> >
> > +static __always_inline void
> > +xdp_reset_data_meta(struct xdp_buff *xdp)
> > +{
> > + xdp->data_meta = xdp->data_hard_start;
> > +}
>
> This is WRONG ... it should be:
>
> xdp->data_meta = xdp->data;
>
maybe the name of the function is not suitable for the use case.
i need to set xdp->data_meta = xdp->data in the driver to start storing
meta data.
^ permalink raw reply
* [PATCH iproute2] man: Fix typos on tc-cbs
From: Jesus Sanchez-Palencia @ 2018-06-27 17:50 UTC (permalink / raw)
To: netdev; +Cc: Jesus Sanchez-Palencia
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
man/man8/tc-cbs.8 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/man/man8/tc-cbs.8 b/man/man8/tc-cbs.8
index 32e1e0d4..ad1d8821 100644
--- a/man/man8/tc-cbs.8
+++ b/man/man8/tc-cbs.8
@@ -28,7 +28,7 @@ defined rate limiting method to the traffic.
This queueing discipline is intended to be used by TSN (Time Sensitive
Networking) applications, the CBS parameters are derived directly by
what is described by the Annex L of the IEEE 802.1Q-2014
-Sepcification. The algorithm and how it affects the latency are
+Specification. The algorithm and how it affects the latency are
detailed there.
CBS is meant to be installed under another qdisc that maps packet
@@ -60,7 +60,7 @@ packet size, which is then used for calculating the idleslope.
sendslope
Sendslope is the rate of credits that is depleted (it should be a
negative number of kilobits per second) when a transmission is
-ocurring. It can be calculated as follows, (IEEE 802.1Q-2014 Section
+occurring. It can be calculated as follows, (IEEE 802.1Q-2014 Section
8.6.8.2 item g):
sendslope = idleslope - port_transmit_rate
--
2.17.1
^ permalink raw reply related
* Re: [RFC bpf-next 6/6] samples/bpf: Add meta data hash example to xdp_redirect_cpu
From: Saeed Mahameed @ 2018-06-27 18:04 UTC (permalink / raw)
To: saeedm@dev.mellanox.co.il, brouer@redhat.com
Cc: alexander.h.duyck@intel.com, peter.waskiewicz.jr@intel.com,
Rony Efraim, borkmann@iogearbox.net, Tariq Toukan,
neerav.parikh@intel.com, Opher Reviv,
alexei.starovoitov@gmail.com, pjwaskiewicz@gmail.com,
netdev@vger.kernel.org, ttoukan.linux@gmail.com
In-Reply-To: <20180627125914.1a3b52db@redhat.com>
On Wed, 2018-06-27 at 12:59 +0200, Jesper Dangaard Brouer wrote:
> On Tue, 26 Jun 2018 19:46:15 -0700
> Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
>
> > Add a new program (prog_num = 4) that will not parse packets and
> > will
> > use the meta data hash to spread/redirect traffic into different
> > cpus.
>
> You cannot "steal" prognum 4, as it is already used for
> "xdp_prognum4_ddos_filter_pktgen". Please append your new prog as
> #5.
>
Sure.
> > For the new program we set on bpf_set_link_xdp_fd:
> > xdp_flags |= XDP_FLAGS_META_HASH | XDP_FLAGS_META_VLAN;
> >
> > On mlx5 it will succeed since mlx5 already supports these flags.
> >
> > The new program will read the value of the hash from the data_meta
> > pointer from the xdp_md and will use it to compute the destination
> > cpu.
> >
> > Note: I didn't test this patch to show redirect works with the
> > hash!
> > I only used it to see that the hash and vlan values are set
> > correctly
> > by the driver and can be seen by the xdp program.
> >
> > * I faced some difficulties to read the hash value using the helper
> > functions defined in the previous patches, but once i used the same
> > logic
> > with out these functions it worked ! Will have to figure this out
> > later.
> >
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> > samples/bpf/xdp_redirect_cpu_kern.c | 67
> > +++++++++++++++++++++++++++++
> > samples/bpf/xdp_redirect_cpu_user.c | 7 +++
> > 2 files changed, 74 insertions(+)
> >
> > diff --git a/samples/bpf/xdp_redirect_cpu_kern.c
> > b/samples/bpf/xdp_redirect_cpu_kern.c
> > index 303e9e7161f3..d6b3f55f342a 100644
> > --- a/samples/bpf/xdp_redirect_cpu_kern.c
> > +++ b/samples/bpf/xdp_redirect_cpu_kern.c
> > @@ -376,6 +376,73 @@ int xdp_prognum3_proto_separate(struct xdp_md
> > *ctx)
> > return bpf_redirect_map(&cpu_map, cpu_dest, 0);
> > }
> >
> > +#if 0
> > +xdp_md_info_arr mdi = {
> > + [XDP_DATA_META_HASH] = {.offset = 0, .present = 1},
> > + [XDP_DATA_META_VLAN] = {.offset = sizeof(struct
> > xdp_md_hash), .present = 1},
> > +};
> > +#endif
>
> Sorry, no global variables avail in the generated BPF byte-code.
>
Yea i found out the hard way :), but for my final solution i would like
to somehow share the same static md info array between netdev and bpf
program, so this code was just experimental.
> > +SEC("xdp_cpu_map4_hash_separate")
> > +int xdp_prognum4_hash_separate(struct xdp_md *ctx)
> > +{
> > + void *data_meta = (void *)(long)ctx->data_meta;
> > + void *data_end = (void *)(long)ctx->data_end;
> > + void *data = (void *)(long)ctx->data;
> > + struct xdp_md_hash *hash;
> > + struct xdp_md_vlan *vlan;
> > + struct datarec *rec;
> > + u32 cpu_dest = 0;
> > + u32 cpu_idx = 0;
> > + u32 *cpu_lookup;
> > + u32 key = 0;
> > +
> > + /* Count RX packet in map */
> > + rec = bpf_map_lookup_elem(&rx_cnt, &key);
> > + if (!rec)
> > + return XDP_ABORTED;
> > + rec->processed++;
> > +
> > + /* for some reason this code fails to be verified */
> > +#if 0
> > + hash = xdp_data_meta_get_hash(mdi, data_meta);
>
> This will not work, because it is not implemented as a proper
> BPF-helper call.
>
> First, you currently store the xdp_md_info_arr inside the driver,
> which
> makes it hard for a helper to access this. For helper access, we
> could
> store this in xdp_rxq_info.
>
Good Idea!
> Second, in your design it looks like you are introducing a helper per
> possible item in xdp_md_info_arr. I think we can reduce this to a
> single helper, that takes a XDP_DATA_META_xxx flag, and returns an
> offset. (The helper could return a direct pointer, but I don't think
> the verfier can handle that, as it need to "see" this is related to
> data_meta pointer, and that we do the proper boundry checks.).
>
We can update the verifier to allow access to any offset between
data_meta and data_meta + offset(last meta data) + sizeof(last meta
data) ?
> The BPF prog already have direct memory access to the data_meta area,
> and all it really need is an offset. Sure, the XDP/bpf programmer
> could just calculate these offsets as constants, and remember to load
> the XDP prog with the flags that corresponds to the calculated
> offsets.
>
> But I think we can do something even smarter...
>
> It should be possible to convert/patch the BPF instructions, of the
> helper call that returns an offset, to instead avoid the call and
> either (1) provide the offset as a constant/IMM or (2) make BPF inst
> doing the lookup in xdp_md_info_arr.
>
I vote (2).
>
> > + if (hash + 1 > data)
> > + return XDP_ABORTED;
> > +
> > + vlan = xdp_data_meta_get_vlan(mdi, data_meta);
> > + if (vlan + 1 > data)
> > + return XDP_ABORTED;
> > +#endif
> > +
> > + /* Work around for the above code */
> > + hash = data_meta; /* since we know hash will appear first
> > */
> > + if (hash + 1 > data)
> > + return XDP_ABORTED;
> > +
> > +#if 0
> > + // Just for testing
> > + /* We know that vlan will appear after the hash */
> > + vlan = (void *)((char *)data_meta + sizeof(*hash));
> > + if (vlan + 1 > data) {
> > + return XDP_ABORTED;
> > + }
> > +#endif
>
>
^ permalink raw reply
* Re: [PATCH v7 09/11] net: pch_gbe: Convert to mdiobus and phylib
From: Paul Burton @ 2018-06-27 18:09 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, David S . Miller
In-Reply-To: <20180627175144.GG885@lunn.ch>
Hi Andrew,
On Wed, Jun 27, 2018 at 07:51:44PM +0200, Andrew Lunn wrote:
> > @@ -5,7 +5,8 @@
> > config PCH_GBE
> > tristate "OKI SEMICONDUCTOR IOH(ML7223/ML7831) GbE"
> > depends on PCI && (X86_32 || COMPILE_TEST)
> > - select MII
> > + select PHYLIB
> > + imply AT803X_PHY if X86_32
> > select PTP_1588_CLOCK_PCH
> > select NET_PTP_CLASSIFY
>
> That is unusual. I don't think any other MAC driver does this.
>
> If the AT803X driver is not available, it will fall back to the
> generic PHY driver. That means RGMII delays will not get set
> correctly, no interrupts, no wol, and no workaround for the 8030.
>
> Are any of these relevant to your board?
Well, my board uses an RTL8211E PHY, doesn't support suspending so WoL
isn't applicable & with this series isn't yet using PHY interrupts
(though that would be ideal as a later addition). So for my board I
enable CONFIG_REALTEK_PHY & I don't have CONFIG_AT8031X_PHY enabled.
It seems the Minnowboard uses the AT8031 PHY, but it's not the only X86
board that includes the EG20T & not all of those use the AT8031. For
example I have access to an Aaeon NanoCOM-TC module[1] which uses the
EG20T & pch_gbe, but its datasheet lists an RTL8211CL PHY (which is
presumably misconfigured by current kernels, though at least for basic
network access is functional).
The idea behind using imply was that it allows kernel configurations
that have up to now only supported the AT8031 PHY via pch_gbe's custom
code to automatically continue to support that PHY, but also allows
support for it to be disabled for systems that do not use that PHY (for
example mine or the Aaeon system).
Would you prefer that the MAC driver instead selects the PHY drivers for
all PHYs known to have been used with the MAC? Or would you be happy if
I added the equivalent in patch 11:
imply REALTEK_PHY if MIPS
Though perhaps REALTEK_PHY would be good to enable for X86_32 too to
cover that Aaeon system...
Thanks,
Paul
[1] http://www.aaeon.com/en/p/com-express-modules-nanocom-tc
^ permalink raw reply
* Re: [PATCH rdma-next 08/12] overflow.h: Add arithmetic shift helper
From: Jason Gunthorpe @ 2018-06-27 18:10 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: Leon Romanovsky, Doug Ledford, Kees Cook, Leon Romanovsky,
RDMA mailing list, Hadar Hen Zion, Matan Barak, Michael J Ruhl,
Noa Osherovich, Raed Salem, Yishai Hadas, Saeed Mahameed,
linux-netdev, linux-kernel
In-Reply-To: <CAKwiHFgchr+6HYOZ4e4e1vzL9cFabe6eonNNM8NTWZypazcuKA@mail.gmail.com>
On Wed, Jun 27, 2018 at 11:36:03AM +0200, Rasmus Villemoes wrote:
> OK. The requirement of everything having the same type for the
> check_*_overflow when gccs builtins are not available was mostly a
> consequence of my inability to implement completely type-generic
> versions (but also to enforce some sanity, so people don't do
> check_add_overflow( s8, size_t, int*)). There's no gcc builtin for
> shift, but if it's relatively simple to one allowing a and *d to have
> different types, then why not. It's of course particularly convenient
> to allow a bare "1" (i.e. int) as a while having *d have some random
> type.
Yes
> Wouldn't check_shift_overflow(-1, 4, &someint) just put -16 in someint
> and report no overflow? That's what I'd expect, if negative values are
> to be supported at all.
I would say that is not a desired outcome, bitshift is defined on
bits, if the caller wanted something defined as signed multiply they
should use multiply.
IMHO, nobody writes 'a << b' expecting sign preservation..
> Well, the types you can check at compile-time, the values not, so you
> still have to define the result, i.e. contents of *d, for negative
> values (even if we decide that "overflow" should always be signalled in
> that case).
Why do a need to define a 'result' beyond whatever the not-undefined
behavior shift expression produces?
> What about more like this?
> check_shift_overflow(a, s, d) ({
> // Shift is always performed on the machine's largest
> unsigned
> u64 _a = a;
> typeof(s) _s = s;
> typeof(d) _d = d;
> // Make s safe against UB
> unsigned int _to_shift = _s >= 0 && _s < 8*sizeof(*d) : _s ? 0;
> *_d = (_a << _to_shift);
> // s is malformed
> (_to_shift != _s ||
> // d is a signed type and became negative
> *_d < 0 ||
> // a is a signed type and was negative
> _a < 0 ||
> // Not invertable means a was truncated during
> shifting
> (*_d >> _to_shift) != a))
> })
> I'm not seeing a UB with this?
>
> Something like that might work, but you're not there yet. In
> particular, your test for whether a is negative is thwarted by using
> u64 for _a and testing _a < 0...
Oops, yes that was intended to be 'a', and of course we need to
capture it..
Leon? Seems like agreement, Can you work with this version?
#include <stdint.h>
#include <stdbool.h>
#include <assert.h>
#define u64 uint64_t
/*
* Compute *d = (a << s)
*
* Returns true if '*d' cannot hold the result or 'a << s' doesn't make sense.
* - 'a << s' causes bits to be lost when stored in d
* - 's' is garbage (eg negative) or so large that a << s is guarenteed to be 0
* - 'a' is negative
* - 'a << s' sets the sign bit, if any, in '*d'
* *d is not defined if false is returned.
*/
#define check_shift_overflow(a, s, d) \
({ \
typeof(a) _a = a; \
typeof(s) _s = s; \
typeof(d) _d = d; \
u64 _a_full = _a; \
unsigned int _to_shift = \
_s >= 0 && _s < 8 * sizeof(*d) ? _s : 0; \
\
*_d = (_a_full << _to_shift); \
\
(_to_shift != _s || *_d < 0 || _a < 0 || \
(*_d >> _to_shift) != a); \
})
int main(int argc, const char *argv[])
{
int32_t s32;
uint32_t u32;
assert(check_shift_overflow(1, 0, &s32) == false && s32 == (1 << 0));
assert(check_shift_overflow(1, 1, &s32) == false && s32 == (1 << 1));
assert(check_shift_overflow(1, 30, &s32) == false && s32 == (1 << 30));
assert(check_shift_overflow(1, 31, &s32) == true);
assert(check_shift_overflow(1, 32, &s32) == true);
assert(check_shift_overflow(-1, 1, &s32) == true);
assert(check_shift_overflow(-1, 0, &s32) == true);
assert(check_shift_overflow(1, 0, &u32) == false && u32 == (1 << 0));
assert(check_shift_overflow(1, 1, &u32) == false && u32 == (1 << 1));
assert(check_shift_overflow(1, 30, &u32) == false && u32 == (1 << 30));
assert(check_shift_overflow(1, 31, &u32) == false && u32 == (1UL << 31));
assert(check_shift_overflow(1, 32, &u32) == true);
assert(check_shift_overflow(-1, 1, &u32) == true);
assert(check_shift_overflow(-1, 0, &u32) == true);
assert(check_shift_overflow(0xFFFFFFFF, 0, &u32) == false && u32 == (0xFFFFFFFFUL << 0));
assert(check_shift_overflow(0xFFFFFFFF, 1, &u32) == true);
assert(check_shift_overflow(0xFFFFFFFF, 0, &s32) == true);
assert(check_shift_overflow(0xFFFFFFFF, 1, &s32) == true);
}
Thanks,
Jason
^ permalink raw reply
* Re: [PATCH v7 06/11] net: pch_gbe: Only enable MAC when PHY link is active
From: Paul Burton @ 2018-06-27 18:15 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, David S . Miller, Andrew Lunn
In-Reply-To: <34bf7133-5f4d-ef61-2448-e1fcc1e3036a@gmail.com>
Hi Florian,
On Wed, Jun 27, 2018 at 10:54:24AM -0700, Florian Fainelli wrote:
> On 06/26/2018 05:06 PM, 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.
>
> What we ended up doing in the bcmgenet driver is loop back the RX and TX
> clocks such that we always have a clock that we can use to perform any
> MAC operation, including reset.
>
> Is this an option here?
That sounds like a nice solution, but I don't see a way to do it in the
EG20T datasheet[1].
> You might also want to split the allocation from the actual
> initialization if this is not done already.
Some of the buffer allocation is still happening in pch_gbe_up() rather
than when the link actually comes up, but there is more that could
probably be moved.
Thanks,
Paul
[1] https://www.intel.com/content/www/us/en/intelligent-systems/queens-bay/platform-controller-hub-eg20t-datasheet.html
^ permalink raw reply
* Re: [PATCH bpf 2/4] xsk: frame could be completed more than once in SKB path
From: Song Liu @ 2018-06-27 18:19 UTC (permalink / raw)
To: Magnus Karlsson
Cc: bjorn.topel, ast, Daniel Borkmann, Networking, qi.z.zhang, pavel
In-Reply-To: <1530108136-4984-3-git-send-email-magnus.karlsson@intel.com>
On Wed, Jun 27, 2018 at 7:02 AM, Magnus Karlsson
<magnus.karlsson@intel.com> wrote:
> Fixed a bug in which a frame could be completed more than once
> when an error was returned from dev_direct_xmit(). The code
> erroneously retried sending the message leading to multiple
> calls to the SKB destructor and therefore multiple completions
> of the same buffer to user space.
>
> The error code in this case has been changed from EAGAIN to EBUSY
> in order to tell user space that the sending of the packet failed
> and the buffer has been return to user space through the completion
> queue.
>
> Fixes: 35fcde7f8deb ("xsk: support for Tx")
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Reported-by: Pavel Odintsov <pavel@fastnetmon.com>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> net/xdp/xsk.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 3b3410ada097..d482f727f4c2 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -268,15 +268,15 @@ static int xsk_generic_xmit(struct sock *sk, struct msghdr *m,
> skb->destructor = xsk_destruct_skb;
>
> err = dev_direct_xmit(skb, xs->queue_id);
> + xskq_discard_desc(xs->tx);
> /* Ignore NET_XMIT_CN as packet might have been sent */
> if (err == NET_XMIT_DROP || err == NETDEV_TX_BUSY) {
> - err = -EAGAIN;
> - /* SKB consumed by dev_direct_xmit() */
> + /* SKB completed but not sent */
> + err = -EBUSY;
> goto out;
> }
>
> sent_frame = true;
> - xskq_discard_desc(xs->tx);
> }
>
> out:
> --
> 2.7.4
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox