* [iproute PATCH v4 3/4] tc/tc_filter: Make sure filter name is not empty
From: Phil Sutter @ 2017-08-24 9:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094634.4093-1-phil@nwl.cc>
The later check for 'k[0] != 0' requires a non-empty filter name,
otherwise NULL pointer dereference in 'q' might happen.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v2:
- Instead of calling strlen(), just make sure **argv is not 0.
---
tc/tc_filter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index b13fb9185d4fd..cf290ae8e252c 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -412,6 +412,9 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
usage();
return 0;
} else {
+ if (!**argv)
+ invarg("invalid filter name", *argv);
+
strncpy(k, *argv, sizeof(k)-1);
q = get_filter_kind(k);
--
2.13.1
^ permalink raw reply related
* [PATCH net-next 4/4] net: mvpp2: fallback using h/w and random mac if the dt one isn't valid
From: Antoine Tenart @ 2017-08-24 9:46 UTC (permalink / raw)
To: davem, thomas.petazzoni
Cc: Antoine Tenart, andrew, gregory.clement, nadavh, linux,
linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170824094658.1296-1-antoine.tenart@free-electrons.com>
When using a mac address described in the device tree, a check is made
to see if it is valid. When it's not, no fallback is defined. This
patches tries to get the mac address from h/w (or use a random one if
the h/w one isn't valid) when the dt mac address isn't valid.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index fe8309124a09..b53254ef7cae 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7465,19 +7465,20 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
if (dt_mac_addr && is_valid_ether_addr(dt_mac_addr)) {
*mac_from = "device tree";
ether_addr_copy(dev->dev_addr, dt_mac_addr);
- } else {
- if (priv->hw_version == MVPP21) {
- mvpp21_get_mac_address(port, hw_mac_addr);
- if (is_valid_ether_addr(hw_mac_addr)) {
- *mac_from = "hardware";
- ether_addr_copy(dev->dev_addr, hw_mac_addr);
- return;
- }
- }
+ return;
+ }
- *mac_from = "random";
- eth_hw_addr_random(dev);
+ if (priv->hw_version == MVPP21) {
+ mvpp21_get_mac_address(port, hw_mac_addr);
+ if (is_valid_ether_addr(hw_mac_addr)) {
+ *mac_from = "hardware";
+ ether_addr_copy(dev->dev_addr, hw_mac_addr);
+ return;
+ }
}
+
+ *mac_from = "random";
+ eth_hw_addr_random(dev);
}
/* Ports initialization */
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 3/4] net: mvpp2: fix use of the random mac address for PPv2.2
From: Antoine Tenart @ 2017-08-24 9:46 UTC (permalink / raw)
To: davem, thomas.petazzoni
Cc: Antoine Tenart, andrew, gregory.clement, nadavh, linux,
linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170824094658.1296-1-antoine.tenart@free-electrons.com>
The MAC retrieval logic is using a variable to store an h/w stored mac
address and checks this mac against invalid ones before using it. But
the mac address is only read from h/w when using PPv2.1. So when using
PPv2.2 it defaults to its init state.
This patches fixes the logic to only check if the h/w mac is valid when
actually retrieving a mac from h/w.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 908e5b148fd7..fe8309124a09 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7466,15 +7466,17 @@ static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
*mac_from = "device tree";
ether_addr_copy(dev->dev_addr, dt_mac_addr);
} else {
- if (priv->hw_version == MVPP21)
+ if (priv->hw_version == MVPP21) {
mvpp21_get_mac_address(port, hw_mac_addr);
- if (is_valid_ether_addr(hw_mac_addr)) {
- *mac_from = "hardware";
- ether_addr_copy(dev->dev_addr, hw_mac_addr);
- } else {
- *mac_from = "random";
- eth_hw_addr_random(dev);
+ if (is_valid_ether_addr(hw_mac_addr)) {
+ *mac_from = "hardware";
+ ether_addr_copy(dev->dev_addr, hw_mac_addr);
+ return;
+ }
}
+
+ *mac_from = "random";
+ eth_hw_addr_random(dev);
}
}
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 2/4] net: mvpp2: move the mac retrieval/copy logic into its own function
From: Antoine Tenart @ 2017-08-24 9:46 UTC (permalink / raw)
To: davem, thomas.petazzoni
Cc: Antoine Tenart, andrew, gregory.clement, nadavh, linux,
linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170824094658.1296-1-antoine.tenart@free-electrons.com>
The MAC retrieval has a quite complicated logic (which is broken). Moves
it to its own function to prepare for patches fixing its logic, so that
reviews are easier.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 45 +++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 50a0920d3282..908e5b148fd7 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7453,6 +7453,31 @@ static bool mvpp2_port_has_tx_irqs(struct mvpp2 *priv,
return true;
}
+static void mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 *priv,
+ struct device_node *port_node,
+ char **mac_from)
+{
+ struct mvpp2_port *port = netdev_priv(dev);
+ char hw_mac_addr[ETH_ALEN] = {0};
+ const char *dt_mac_addr;
+
+ dt_mac_addr = of_get_mac_address(port_node);
+ if (dt_mac_addr && is_valid_ether_addr(dt_mac_addr)) {
+ *mac_from = "device tree";
+ ether_addr_copy(dev->dev_addr, dt_mac_addr);
+ } else {
+ if (priv->hw_version == MVPP21)
+ mvpp21_get_mac_address(port, hw_mac_addr);
+ if (is_valid_ether_addr(hw_mac_addr)) {
+ *mac_from = "hardware";
+ ether_addr_copy(dev->dev_addr, hw_mac_addr);
+ } else {
+ *mac_from = "random";
+ eth_hw_addr_random(dev);
+ }
+ }
+}
+
/* Ports initialization */
static int mvpp2_port_probe(struct platform_device *pdev,
struct device_node *port_node,
@@ -7464,9 +7489,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
struct mvpp2_port_pcpu *port_pcpu;
struct net_device *dev;
struct resource *res;
- const char *dt_mac_addr;
- const char *mac_from;
- char hw_mac_addr[ETH_ALEN] = {0};
+ char *mac_from = "";
unsigned int ntxqs, nrxqs;
bool has_tx_irqs;
u32 id;
@@ -7575,21 +7598,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
goto err_free_irq;
}
- dt_mac_addr = of_get_mac_address(port_node);
- if (dt_mac_addr && is_valid_ether_addr(dt_mac_addr)) {
- mac_from = "device tree";
- ether_addr_copy(dev->dev_addr, dt_mac_addr);
- } else {
- if (priv->hw_version == MVPP21)
- mvpp21_get_mac_address(port, hw_mac_addr);
- if (is_valid_ether_addr(hw_mac_addr)) {
- mac_from = "hardware";
- ether_addr_copy(dev->dev_addr, hw_mac_addr);
- } else {
- mac_from = "random";
- eth_hw_addr_random(dev);
- }
- }
+ mvpp2_port_copy_mac_addr(dev, priv, port_node, &mac_from);
port->tx_ring_size = MVPP2_MAX_TXD;
port->rx_ring_size = MVPP2_MAX_RXD;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 1/4] net: mvpp2: fix the mac address used when using PPv2.2
From: Antoine Tenart @ 2017-08-24 9:46 UTC (permalink / raw)
To: davem, thomas.petazzoni
Cc: Antoine Tenart, andrew, gregory.clement, nadavh, linux,
linux-kernel, mw, stefanc, netdev
In-Reply-To: <20170824094658.1296-1-antoine.tenart@free-electrons.com>
The mac address is only retrieved from h/w when using PPv2.1. Otherwise
the variable holding it is still checked and used if it contains a valid
value. As the variable isn't initialized to an invalid mac address
value, we end up with random mac addresses which can be the same for all
the ports handled by this PPv2 driver.
Fixes this by initializing the h/w mac address variable to {0}, which is
an invalid mac address value. This way the random assignation fallback
is called and all ports end up with their own addresses.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Fixes: 2697582144dd ("net: mvpp2: handle misc PPv2.1/PPv2.2 differences")
---
drivers/net/ethernet/marvell/mvpp2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 3f8cbc070dc4..50a0920d3282 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -7466,7 +7466,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
struct resource *res;
const char *dt_mac_addr;
const char *mac_from;
- char hw_mac_addr[ETH_ALEN];
+ char hw_mac_addr[ETH_ALEN] = {0};
unsigned int ntxqs, nrxqs;
bool has_tx_irqs;
u32 id;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 0/4] net: mvpp2: fix the mac address retrieval logic
From: Antoine Tenart @ 2017-08-24 9:46 UTC (permalink / raw)
To: davem, thomas.petazzoni
Cc: Antoine Tenart, andrew, gregory.clement, nadavh, linux,
linux-kernel, mw, stefanc, netdev
Hi all,
The MAC address retrieval logic was broken and when using the PPv2
driver on PPv2.2 engines I ended up using the same mac address on all
ports. This series of patches fixes this, and also tackle a possible bug
when defining the mac address in the device tree.
To fix this in a nice way I ended up using a dedicated function to
handle the mac retrieval logic. This can be hard to backport into stable
kernels. This is why I also made a quick fix which is easy to backport
(patch 1/14), to tackle down the PPv2.2 mac retrieval bug. Let me know
if this approach is the proper way to handle this or if I should do
something else.
Thanks!
Antoine
Fixes: 2697582144dd ("net: mvpp2: handle misc PPv2.1/PPv2.2 differences")
Antoine Tenart (4):
net: mvpp2: fix the mac address used when using PPv2.2
net: mvpp2: move the mac retrieval/copy logic into its own function
net: mvpp2: fix use of the random mac address for PPv2.2
net: mvpp2: fallback using h/w and random mac if the dt one isn't
valid
drivers/net/ethernet/marvell/mvpp2.c | 48 ++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 18 deletions(-)
--
2.13.5
^ permalink raw reply
* [iproute PATCH v4 0/4] Covscan: Fix potential NULL pointer dereferences
From: Phil Sutter @ 2017-08-24 9:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
This series collects patches from v1 which eliminate possible cases of
NULL pointer dereferences.
Changes since v3:
- Dropped upstream rejected patch 2.
Phil Sutter (4):
ifstat, nstat: Check fdopen() return value
tc/q_netem: Don't dereference possibly NULL pointer
tc/tc_filter: Make sure filter name is not empty
tipc/bearer: Prevent NULL pointer dereference
misc/ifstat.c | 16 +++++++++++-----
misc/nstat.c | 16 +++++++++++-----
tc/q_netem.c | 3 ++-
tc/tc_filter.c | 3 +++
tipc/bearer.c | 2 +-
5 files changed, 28 insertions(+), 12 deletions(-)
--
2.13.1
^ permalink raw reply
* [iproute PATCH v4 2/4] tc/q_netem: Don't dereference possibly NULL pointer
From: Phil Sutter @ 2017-08-24 9:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094634.4093-1-phil@nwl.cc>
Assuming 'opt' might be NULL, move the call to RTA_PAYLOAD to after the
check since it dereferences its parameter.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v2:
- Dropped empty line between assignment and check.
---
tc/q_netem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tc/q_netem.c b/tc/q_netem.c
index 0975ae111de97..5a9e747411e85 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -538,7 +538,7 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
int *ecn = NULL;
struct tc_netem_qopt qopt;
const struct tc_netem_rate *rate = NULL;
- int len = RTA_PAYLOAD(opt) - sizeof(qopt);
+ int len;
__u64 rate64 = 0;
SPRINT_BUF(b1);
@@ -546,6 +546,7 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
if (opt == NULL)
return 0;
+ len = RTA_PAYLOAD(opt) - sizeof(qopt);
if (len < 0) {
fprintf(stderr, "options size error\n");
return -1;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH v3 4/6] lib/bpf: Check return value of write()
From: Phil Sutter @ 2017-08-24 9:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094131.2963-1-phil@nwl.cc>
This is merely to silence the compiler warning. If write to stderr
failed, assume that printing an error message will fail as well so don't
even try.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
lib/bpf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index 1dcb261dc915f..825e071cea572 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -591,7 +591,8 @@ int bpf_trace_pipe(void)
ret = read(fd, buff, sizeof(buff) - 1);
if (ret > 0) {
- write(2, buff, ret);
+ if (write(STDERR_FILENO, buff, ret) != ret)
+ return -1;
fflush(stderr);
}
}
--
2.13.1
^ permalink raw reply related
* [iproute PATCH v3 5/6] lib/fs: Fix and simplify make_path()
From: Phil Sutter @ 2017-08-24 9:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094131.2963-1-phil@nwl.cc>
Calling stat() before mkdir() is racey: The entry might change in
between. Also, the call to stat() seems to exist only to check if the
directory exists already. So simply call mkdir() unconditionally and
catch only errors other than EEXIST.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
lib/fs.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/lib/fs.c b/lib/fs.c
index c59ac564581d0..7083bf2e23bb7 100644
--- a/lib/fs.c
+++ b/lib/fs.c
@@ -102,7 +102,6 @@ out:
int make_path(const char *path, mode_t mode)
{
char *dir, *delim;
- struct stat sbuf;
int rc = -1;
delim = dir = strdup(path);
@@ -120,20 +119,11 @@ int make_path(const char *path, mode_t mode)
if (delim)
*delim = '\0';
- if (stat(dir, &sbuf) != 0) {
- if (errno != ENOENT) {
- fprintf(stderr,
- "stat failed for %s: %s\n",
- dir, strerror(errno));
- goto out;
- }
-
- if (mkdir(dir, mode) != 0) {
- fprintf(stderr,
- "mkdir failed for %s: %s\n",
- dir, strerror(errno));
- goto out;
- }
+ rc = mkdir(dir, mode);
+ if (mkdir(dir, mode) != 0 && errno != EEXIST) {
+ fprintf(stderr, "mkdir failed for %s: %s\n",
+ dir, strerror(errno));
+ goto out;
}
if (delim == NULL)
--
2.13.1
^ permalink raw reply related
* [iproute PATCH v3 0/6] Covscan: Misc fixes
From: Phil Sutter @ 2017-08-24 9:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
This series collects patches from v1 addressing miscellaneous issues
detected by covscan.
Changes since v2:
- Dropped patch 1 since v2 discussion is still inconclusive.
- Replaced patch 2 by a more appropriate one given feedback from v2.
Phil Sutter (6):
ss: Make struct tcpstat fields 'timer' and 'timeout' unsigned
ss: Make sure scanned index value to unix_state_map is sane
netem/maketable: Check return value of fscanf()
lib/bpf: Check return value of write()
lib/fs: Fix and simplify make_path()
lib/libnetlink: Don't pass NULL parameter to memcpy()
lib/bpf.c | 3 ++-
lib/fs.c | 20 +++++---------------
lib/libnetlink.c | 6 ++++--
misc/ss.c | 11 +++++------
netem/maketable.c | 4 ++--
5 files changed, 18 insertions(+), 26 deletions(-)
--
2.13.1
^ permalink raw reply
* [iproute PATCH v3 2/6] ss: Make sure scanned index value to unix_state_map is sane
From: Phil Sutter @ 2017-08-24 9:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094131.2963-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ss.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/misc/ss.c b/misc/ss.c
index c41d5169aba52..951aa877bcb01 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3148,7 +3148,8 @@ static int unix_show(struct filter *f)
if (flags & (1 << 16)) {
u->state = SS_LISTEN;
- } else {
+ } else if (u->state > 0 &&
+ u->state <= ARRAY_SIZE(unix_state_map)) {
u->state = unix_state_map[u->state-1];
if (u->type == SOCK_DGRAM && u->state == SS_CLOSE && u->rport)
u->state = SS_ESTABLISHED;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH v3 3/6] netem/maketable: Check return value of fscanf()
From: Phil Sutter @ 2017-08-24 9:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094131.2963-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
netem/maketable.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/netem/maketable.c b/netem/maketable.c
index ad660e7d457f0..ccb8f0c68b062 100644
--- a/netem/maketable.c
+++ b/netem/maketable.c
@@ -38,8 +38,8 @@ readdoubles(FILE *fp, int *number)
}
for (i=0; i<limit; ++i){
- fscanf(fp, "%lf", &x[i]);
- if (feof(fp))
+ if (fscanf(fp, "%lf", &x[i]) != 1 ||
+ feof(fp))
break;
++n;
}
--
2.13.1
^ permalink raw reply related
* [iproute PATCH v3 1/6] ss: Make struct tcpstat fields 'timer' and 'timeout' unsigned
From: Phil Sutter @ 2017-08-24 9:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094131.2963-1-phil@nwl.cc>
Both 'timer' and 'timeout' variables of struct tcpstat are either
scanned as unsigned values from /proc/net/tcp{,6} or copied from
'idiag_timer' and 'idiag_expries' fields of struct inet_diag_msg, which
itself are unsigned. Therefore they may be unsigned as well, which
eliminates the need to check for negative values.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/ss.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index 34c6da5443642..c41d5169aba52 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -697,8 +697,8 @@ struct dctcpstat {
struct tcpstat {
struct sockstat ss;
- int timer;
- int timeout;
+ unsigned int timer;
+ unsigned int timeout;
int probes;
char cong_alg[16];
double rto, ato, rtt, rttvar;
@@ -869,13 +869,11 @@ static void sock_addr_print(const char *addr, char *delim, const char *port,
sock_addr_print_width(addr_width, addr, delim, serv_width, port, ifname);
}
-static const char *print_ms_timer(int timeout)
+static const char *print_ms_timer(unsigned int timeout)
{
static char buf[64];
int secs, msecs, minutes;
- if (timeout < 0)
- timeout = 0;
secs = timeout/1000;
minutes = secs/60;
secs = secs%60;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH v3 6/6] lib/libnetlink: Don't pass NULL parameter to memcpy()
From: Phil Sutter @ 2017-08-24 9:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170824094131.2963-1-phil@nwl.cc>
Both addattr_l() and rta_addattr_l() may be called with NULL data
pointer and 0 alen parameters. Avoid calling memcpy() in that case.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
lib/libnetlink.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 874e660be7eb4..fbe719ee10449 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -871,7 +871,8 @@ int addattr_l(struct nlmsghdr *n, int maxlen, int type, const void *data,
rta = NLMSG_TAIL(n);
rta->rta_type = type;
rta->rta_len = len;
- memcpy(RTA_DATA(rta), data, alen);
+ if (alen)
+ memcpy(RTA_DATA(rta), data, alen);
n->nlmsg_len = NLMSG_ALIGN(n->nlmsg_len) + RTA_ALIGN(len);
return 0;
}
@@ -958,7 +959,8 @@ int rta_addattr_l(struct rtattr *rta, int maxlen, int type,
subrta = (struct rtattr *)(((char *)rta) + RTA_ALIGN(rta->rta_len));
subrta->rta_type = type;
subrta->rta_len = len;
- memcpy(RTA_DATA(subrta), data, alen);
+ if (alen)
+ memcpy(RTA_DATA(subrta), data, alen);
rta->rta_len = NLMSG_ALIGN(rta->rta_len) + RTA_ALIGN(len);
return 0;
}
--
2.13.1
^ permalink raw reply related
* [PATCH net-next v1] net: gso: Add GSO support for NSH (Network Service Header)
From: Yi Yang @ 2017-08-24 9:36 UTC (permalink / raw)
To: netdev; +Cc: jbenc, simon.horman, Yi Yang
NSH (Network Service Header)[1] is a new protocol for service
function chaining, it can be handled as a L3 protocol like
IPv4 and IPv6, Eth + NSH + Inner packet or VxLAN-gpe + NSH
+ Inner packet are two typical use cases.
We need to enbale Open vSwitch to support NSH, this patch
is to make Linux network infrastructure able to support
NSH GSO for big packet.
[1] https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/
Signed-off-by: Yi Yang <yi.y.yang@intel.com>
---
include/net/nsh.h | 307 ++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/if_ether.h | 1 +
net/Kconfig | 1 +
net/Makefile | 1 +
net/nsh/Kconfig | 10 ++
net/nsh/Makefile | 4 +
net/nsh/nsh_gso.c | 116 ++++++++++++++++
7 files changed, 440 insertions(+)
create mode 100644 include/net/nsh.h
create mode 100644 net/nsh/Kconfig
create mode 100644 net/nsh/Makefile
create mode 100644 net/nsh/nsh_gso.c
diff --git a/include/net/nsh.h b/include/net/nsh.h
new file mode 100644
index 0000000..abc4fd4
--- /dev/null
+++ b/include/net/nsh.h
@@ -0,0 +1,307 @@
+#ifndef __NET_NSH_H
+#define __NET_NSH_H 1
+
+/*
+ * Network Service Header:
+ * 0 1 2 3
+ * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Ver|O|U| TTL | Length |U|U|U|U|MD Type| Next Protocol |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Service Path Identifier (SPI) | Service Index |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | |
+ * ~ Mandatory/Optional Context Headers ~
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * Version: The version field is used to ensure backward compatibility
+ * going forward with future NSH specification updates. It MUST be set
+ * to 0x0 by the sender, in this first revision of NSH. Given the
+ * widespread implementation of existing hardware that uses the first
+ * nibble after an MPLS label stack for ECMP decision processing, this
+ * document reserves version 01b and this value MUST NOT be used in
+ * future versions of the protocol. Please see [RFC7325] for further
+ * discussion of MPLS-related forwarding requirements.
+ *
+ * O bit: Setting this bit indicates an Operations, Administration, and
+ * Maintenance (OAM) packet. The actual format and processing of SFC
+ * OAM packets is outside the scope of this specification (see for
+ * example [I-D.ietf-sfc-oam-framework] for one approach).
+ *
+ * The O bit MUST be set for OAM packets and MUST NOT be set for non-OAM
+ * packets. The O bit MUST NOT be modified along the SFP.
+ *
+ * SF/SFF/SFC Proxy/Classifier implementations that do not support SFC
+ * OAM procedures SHOULD discard packets with O bit set, but MAY support
+ * a configurable parameter to enable forwarding received SFC OAM
+ * packets unmodified to the next element in the chain. Forwarding OAM
+ * packets unmodified by SFC elements that do not support SFC OAM
+ * procedures may be acceptable for a subset of OAM functions, but can
+ * result in unexpected outcomes for others, thus it is recommended to
+ * analyze the impact of forwarding an OAM packet for all OAM functions
+ * prior to enabling this behavior. The configurable parameter MUST be
+ * disabled by default.
+ *
+ * TTL: Indicates the maximum SFF hops for an SFP. This field is used
+ * for service plane loop detection. The initial TTL value SHOULD be
+ * configurable via the control plane; the configured initial value can
+ * be specific to one or more SFPs. If no initial value is explicitly
+ * provided, the default initial TTL value of 63 MUST be used. Each SFF
+ * involved in forwarding an NSH packet MUST decrement the TTL value by
+ * 1 prior to NSH forwarding lookup. Decrementing by 1 from an incoming
+ * value of 0 shall result in a TTL value of 63. The packet MUST NOT be
+ * forwarded if TTL is, after decrement, 0.
+ *
+ * All other flag fields, marked U, are unassigned and available for
+ * future use, see Section 11.2.1. Unassigned bits MUST be set to zero
+ * upon origination, and MUST be ignored and preserved unmodified by
+ * other NSH supporting elements. Elements which do not understand the
+ * meaning of any of these bits MUST NOT modify their actions based on
+ * those unknown bits.
+ *
+ * Length: The total length, in 4-byte words, of NSH including the Base
+ * Header, the Service Path Header, the Fixed Length Context Header or
+ * Variable Length Context Header(s). The length MUST be 0x6 for MD
+ * Type equal to 0x1, and MUST be 0x2 or greater for MD Type equal to
+ * 0x2. The length of the NSH header MUST be an integer multiple of 4
+ * bytes, thus variable length metadata is always padded out to a
+ * multiple of 4 bytes.
+ *
+ * MD Type: Indicates the format of NSH beyond the mandatory Base Header
+ * and the Service Path Header. MD Type defines the format of the
+ * metadata being carried.
+ *
+ * 0x0 - This is a reserved value. Implementations SHOULD silently
+ * discard packets with MD Type 0x0.
+ *
+ * 0x1 - This indicates that the format of the header includes a fixed
+ * length Context Header (see Figure 4 below).
+ *
+ * 0x2 - This does not mandate any headers beyond the Base Header and
+ * Service Path Header, but may contain optional variable length Context
+ * Header(s). The semantics of the variable length Context Header(s)
+ * are not defined in this document. The format of the optional
+ * variable length Context Headers is provided in Section 2.5.1.
+ *
+ * 0xF - This value is reserved for experimentation and testing, as per
+ * [RFC3692]. Implementations not explicitly configured to be part of
+ * an experiment SHOULD silently discard packets with MD Type 0xF.
+ *
+ * Next Protocol: indicates the protocol type of the encapsulated data.
+ * NSH does not alter the inner payload, and the semantics on the inner
+ * protocol remain unchanged due to NSH service function chaining.
+ * Please see the IANA Considerations section below, Section 11.2.5.
+ *
+ * This document defines the following Next Protocol values:
+ *
+ * 0x1: IPv4
+ * 0x2: IPv6
+ * 0x3: Ethernet
+ * 0x4: NSH
+ * 0x5: MPLS
+ * 0xFE: Experiment 1
+ * 0xFF: Experiment 2
+ *
+ * Packets with Next Protocol values not supported SHOULD be silently
+ * dropped by default, although an implementation MAY provide a
+ * configuration parameter to forward them. Additionally, an
+ * implementation not explicitly configured for a specific experiment
+ * [RFC3692] SHOULD silently drop packets with Next Protocol values 0xFE
+ * and 0xFF.
+ *
+ * Service Path Identifier (SPI): Identifies a service path.
+ * Participating nodes MUST use this identifier for Service Function
+ * Path selection. The initial classifier MUST set the appropriate SPI
+ * for a given classification result.
+ *
+ * Service Index (SI): Provides location within the SFP. The initial
+ * classifier for a given SFP SHOULD set the SI to 255, however the
+ * control plane MAY configure the initial value of SI as appropriate
+ * (i.e., taking into account the length of the service function path).
+ * The Service Index MUST be decremented by a value of 1 by Service
+ * Functions or by SFC Proxy nodes after performing required services
+ * and the new decremented SI value MUST be used in the egress packet's
+ * NSH. The initial Classifier MUST send the packet to the first SFF in
+ * the identified SFP for forwarding along an SFP. If re-classification
+ * occurs, and that re-classification results in a new SPI, the
+ * (re)classifier is, in effect, the initial classifier for the
+ * resultant SPI.
+ *
+ * The SI is used in conjunction the with Service Path Identifier for
+ * Service Function Path Selection and for determining the next SFF/SF
+ * in the path. The SI is also valuable when troubleshooting or
+ * reporting service paths. Additionally, while the TTL field is the
+ * main mechanism for service plane loop detection, the SI can also be
+ * used for detecting service plane loops.
+ *
+ * When the Base Header specifies MD Type = 0x1, a Fixed Length Context
+ * Header (16-bytes) MUST be present immediately following the Service
+ * Path Header. The value of a Fixed Length Context
+ * Header that carries no metadata MUST be set to zero.
+ *
+ * When the base header specifies MD Type = 0x2, zero or more Variable
+ * Length Context Headers MAY be added, immediately following the
+ * Service Path Header (see Figure 5). Therefore, Length = 0x2,
+ * indicates that only the Base Header followed by the Service Path
+ * Header are present. The optional Variable Length Context Headers
+ * MUST be of an integer number of 4-bytes. The base header Length
+ * field MUST be used to determine the offset to locate the original
+ * packet or frame for SFC nodes that require access to that
+ * information.
+ *
+ * The format of the optional variable length Context Headers
+ *
+ * 0 1 2 3
+ * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Metadata Class | Type |U| Length |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Variable Metadata |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ *
+ * Metadata Class (MD Class): Defines the scope of the 'Type' field to
+ * provide a hierarchical namespace. The IANA Considerations
+ * Section 11.2.4 defines how the MD Class values can be allocated to
+ * standards bodies, vendors, and others.
+ *
+ * Type: Indicates the explicit type of metadata being carried. The
+ * definition of the Type is the responsibility of the MD Class owner.
+ *
+ * Unassigned bit: One unassigned bit is available for future use. This
+ * bit MUST NOT be set, and MUST be ignored on receipt.
+ *
+ * Length: Indicates the length of the variable metadata, in bytes. In
+ * case the metadata length is not an integer number of 4-byte words,
+ * the sender MUST add pad bytes immediately following the last metadata
+ * byte to extend the metadata to an integer number of 4-byte words.
+ * The receiver MUST round up the length field to the nearest 4-byte
+ * word boundary, to locate and process the next field in the packet.
+ * The receiver MUST access only those bytes in the metadata indicated
+ * by the length field (i.e., actual number of bytes) and MUST ignore
+ * the remaining bytes up to the nearest 4-byte word boundary. The
+ * Length may be 0 or greater.
+ *
+ * A value of 0 denotes a Context Header without a Variable Metadata
+ * field.
+ *
+ * [0] https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/
+ */
+
+/**
+ * struct nsh_md1_ctx - Keeps track of NSH context data
+ * @nshc<1-4>: NSH Contexts.
+ */
+struct nsh_md1_ctx {
+ __be32 context[4];
+};
+
+struct nsh_md2_tlv {
+ __be16 md_class;
+ u8 type;
+ u8 length;
+ u8 md_value[];
+};
+
+struct nsh_hdr {
+ __be16 ver_flags_ttl_len;
+ u8 md_type;
+ u8 next_proto;
+ __be32 path_hdr;
+ union {
+ struct nsh_md1_ctx md1;
+ struct nsh_md2_tlv md2;
+ };
+};
+
+/* Masking NSH header fields. */
+#define NSH_VER_MASK 0xc000
+#define NSH_VER_SHIFT 14
+#define NSH_FLAGS_MASK 0x3000
+#define NSH_FLAGS_SHIFT 12
+#define NSH_TTL_MASK 0x0fc0
+#define NSH_TTL_SHIFT 6
+#define NSH_LEN_MASK 0x003f
+#define NSH_LEN_SHIFT 0
+
+#define NSH_MDTYPE_MASK 0x0f
+#define NSH_MDTYPE_SHIFT 0
+
+#define NSH_SPI_MASK 0xffffff00
+#define NSH_SPI_SHIFT 8
+#define NSH_SI_MASK 0x000000ff
+#define NSH_SI_SHIFT 0
+
+/* NSH Base Header Next Protocol. */
+#define NSH_P_IPV4 0x01
+#define NSH_P_IPV6 0x02
+#define NSH_P_ETHERNET 0x03
+#define NSH_P_NSH 0x04
+#define NSH_P_MPLS 0x05
+
+/* MD Type Registry. */
+#define NSH_M_TYPE1 0x01
+#define NSH_M_TYPE2 0x02
+#define NSH_M_EXP1 0xFE
+#define NSH_M_EXP2 0xFF
+
+/* NSH Base Header Length */
+#define NSH_BASE_HDR_LEN 8
+
+/* NSH MD Type 1 header Length. */
+#define NSH_M_TYPE1_LEN 24
+
+/* NSH MD Type 2 header maximum Length. */
+#define NSH_M_TYPE2_MAX_LEN 256
+
+/* NSH MD Type 2 Metadata maximum Length. */
+#define NSH_M_TYPE2_MD_MAX_LEN (NSH_M_TYPE2_MAX_LEN - NSH_BASE_HDR_LEN)
+
+static inline u16 nsh_hdr_len(const struct nsh_hdr *nsh)
+{
+ return ((ntohs(nsh->ver_flags_ttl_len) & NSH_LEN_MASK)
+ >> NSH_LEN_SHIFT) << 2;
+}
+
+static inline u8 nsh_get_ver(const struct nsh_hdr *nsh)
+{
+ return (ntohs(nsh->ver_flags_ttl_len) & NSH_VER_MASK)
+ >> NSH_VER_SHIFT;
+}
+
+static inline u8 nsh_get_flags(const struct nsh_hdr *nsh)
+{
+ return (ntohs(nsh->ver_flags_ttl_len) & NSH_FLAGS_MASK)
+ >> NSH_FLAGS_SHIFT;
+}
+
+static inline u8 nsh_get_ttl(const struct nsh_hdr *nsh)
+{
+ return (ntohs(nsh->ver_flags_ttl_len) & NSH_TTL_MASK)
+ >> NSH_TTL_SHIFT;
+}
+
+static inline void __nsh_set_xflag(struct nsh_hdr *nsh, u16 xflag, u16 xmask)
+{
+ nsh->ver_flags_ttl_len
+ = (nsh->ver_flags_ttl_len & ~htons(xmask)) | htons(xflag);
+}
+
+static inline void nsh_set_flags_and_ttl(struct nsh_hdr *nsh, u8 flags, u8 ttl)
+{
+ __nsh_set_xflag(nsh, ((flags << NSH_FLAGS_SHIFT) & NSH_FLAGS_MASK) |
+ ((ttl << NSH_TTL_SHIFT) & NSH_TTL_MASK),
+ NSH_FLAGS_MASK | NSH_TTL_MASK);
+}
+
+static inline void nsh_set_flags_ttl_len(struct nsh_hdr *nsh, u8 flags,
+ u8 ttl, u8 len)
+{
+ len = len >> 2;
+ __nsh_set_xflag(nsh, ((flags << NSH_FLAGS_SHIFT) & NSH_FLAGS_MASK) |
+ ((ttl << NSH_TTL_SHIFT) & NSH_TTL_MASK) |
+ ((len << NSH_LEN_SHIFT) & NSH_LEN_MASK),
+ NSH_FLAGS_MASK | NSH_TTL_MASK | NSH_LEN_MASK);
+}
+
+#endif /* __NET_NSH_H */
diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
index efeb119..bc5b727 100644
--- a/include/uapi/linux/if_ether.h
+++ b/include/uapi/linux/if_ether.h
@@ -138,6 +138,7 @@
#define ETH_P_IEEE802154 0x00F6 /* IEEE802.15.4 frame */
#define ETH_P_CAIF 0x00F7 /* ST-Ericsson CAIF protocol */
#define ETH_P_XDSA 0x00F8 /* Multiplexed DSA protocol */
+#define ETH_P_NSH 0x894F /* Ethertype for NSH. */
/*
* This is an Ethernet frame header.
diff --git a/net/Kconfig b/net/Kconfig
index 7d57ef3..818df7a 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -240,6 +240,7 @@ source "net/switchdev/Kconfig"
source "net/l3mdev/Kconfig"
source "net/qrtr/Kconfig"
source "net/ncsi/Kconfig"
+source "net/nsh/Kconfig"
config RPS
bool
diff --git a/net/Makefile b/net/Makefile
index bed80fa..82bfac6 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -85,3 +85,4 @@ obj-y += l3mdev/
endif
obj-$(CONFIG_QRTR) += qrtr/
obj-$(CONFIG_NET_NCSI) += ncsi/
+obj-$(CONFIG_NET_NSH_GSO) += nsh/
diff --git a/net/nsh/Kconfig b/net/nsh/Kconfig
new file mode 100644
index 0000000..f2761e7
--- /dev/null
+++ b/net/nsh/Kconfig
@@ -0,0 +1,10 @@
+#
+# NSH GSO support
+#
+
+config NET_NSH_GSO
+ bool "NSH GSO support"
+ depends on INET
+ default y
+ ---help---
+ This allows segmentation of GSO packet that have had NSH header pushed onto them and thus become NSH GSO packets.
diff --git a/net/nsh/Makefile b/net/nsh/Makefile
new file mode 100644
index 0000000..eb4bca0
--- /dev/null
+++ b/net/nsh/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for NSH GSO.
+#
+obj-$(CONFIG_NET_NSH_GSO) += nsh_gso.o
diff --git a/net/nsh/nsh_gso.c b/net/nsh/nsh_gso.c
new file mode 100644
index 0000000..7edc77d
--- /dev/null
+++ b/net/nsh/nsh_gso.c
@@ -0,0 +1,116 @@
+/*
+ * NSH GSO Support
+ *
+ * Authors: Yi Yang (yi.y.yang@intel.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Based on: net/mpls/mpls_gso.c
+ */
+
+#include <linux/err.h>
+#include <linux/netdev_features.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <net/protocol.h>
+#include <net/nsh.h>
+
+static struct sk_buff *nsh_gso_segment(struct sk_buff *skb,
+ netdev_features_t features)
+{
+ struct sk_buff *segs = ERR_PTR(-EINVAL);
+ __be16 protocol = skb->protocol;
+ __be16 inner_proto;
+ u16 mac_offset = skb->mac_header;
+ u16 mac_len = skb->mac_len;
+ struct nsh_hdr *nsh;
+ unsigned int nsh_hlen;
+ const struct net_offload *ops;
+ struct sk_buff *(*gso_inner_segment)(struct sk_buff *skb,
+ netdev_features_t features);
+
+ skb_reset_network_header(skb);
+ nsh = (struct nsh_hdr *)skb_network_header(skb);
+ nsh_hlen = nsh_hdr_len(nsh);
+ if (unlikely(!pskb_may_pull(skb, nsh_hlen)))
+ goto out;
+
+ __skb_pull(skb, nsh_hlen);
+
+ skb_reset_mac_header(skb);
+ skb_reset_mac_len(skb);
+
+ rcu_read_lock();
+ switch (nsh->next_proto) {
+ case NSH_P_ETHERNET:
+ inner_proto = htons(ETH_P_TEB);
+ gso_inner_segment = skb_mac_gso_segment;
+ break;
+ case NSH_P_IPV4:
+ inner_proto = htons(ETH_P_IP);
+ ops = rcu_dereference(inet_offloads[inner_proto]);
+ if (!ops || !ops->callbacks.gso_segment)
+ goto out;
+ gso_inner_segment = ops->callbacks.gso_segment;
+ break;
+ case NSH_P_IPV6:
+ inner_proto = htons(ETH_P_IPV6);
+ ops = rcu_dereference(inet6_offloads[inner_proto]);
+ if (!ops || !ops->callbacks.gso_segment)
+ goto out;
+ gso_inner_segment = ops->callbacks.gso_segment;
+ break;
+ case NSH_P_NSH:
+ inner_proto = htons(ETH_P_NSH);
+ gso_inner_segment = nsh_gso_segment;
+ break;
+ default:
+ skb_gso_error_unwind(skb, protocol, nsh_hlen, mac_offset,
+ mac_len);
+ goto out;
+ }
+
+ skb->protocol = inner_proto;
+ segs = gso_inner_segment(skb, features);
+ if (IS_ERR_OR_NULL(segs)) {
+ skb_gso_error_unwind(skb, protocol, nsh_hlen, mac_offset,
+ mac_len);
+ goto out;
+ }
+
+ do {
+ skb->mac_len = mac_len;
+ skb->protocol = protocol;
+
+ skb_reset_inner_network_header(skb);
+
+ __skb_push(skb, nsh_hlen);
+
+ skb_reset_mac_header(skb);
+ skb_set_network_header(skb, mac_len);
+ } while ((skb = skb->next));
+
+out:
+ rcu_read_unlock();
+ return segs;
+}
+
+static struct packet_offload nsh_offload __read_mostly = {
+ .type = cpu_to_be16(ETH_P_NSH),
+ .priority = 15,
+ .callbacks = {
+ .gso_segment = nsh_gso_segment,
+ },
+};
+
+static int __init nsh_gso_init(void)
+{
+ dev_add_offload(&nsh_offload);
+
+ return 0;
+}
+
+fs_initcall(nsh_gso_init);
--
2.5.5
^ permalink raw reply related
* Re: [PATCH][next] net: hinic: fix comparison of a uint16_t type with -1
From: Aviad Krawczyk @ 2017-08-24 9:38 UTC (permalink / raw)
To: Dan Carpenter, Colin Ian King; +Cc: netdev, kernel-janitors, linux-kernel
In-Reply-To: <20170824092951.ozllwwklfld44u5u@mwanda>
On 8/24/2017 12:29 PM, Dan Carpenter wrote:
> On Thu, Aug 24, 2017 at 09:54:03AM +0100, Colin Ian King wrote:
>> On 24/08/17 09:48, Aviad Krawczyk wrote:
>>> On 8/23/2017 6:39 PM, Colin King wrote:
>>>> From: Colin Ian King <colin.king@canonical.com>
>>>>
>>>> The comparison of hw_ioctxt.rx_buf_sz_idx == -1 is always false because
>>>> rx_buf_sz_idx is a uint16_t. Fix this by explicitly casting -1 to uint16_t.
>>>>
>>>> Detected by CoverityScan, CID#1454559 ("Operands don't affect result")
>>>>
>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>> ---
>>>> drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
>>>> index 09dec6de8dd5..71e26070fb7f 100644
>>>> --- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
>>>> +++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
>>>> @@ -352,7 +352,7 @@ static int set_hw_ioctxt(struct hinic_hwdev *hwdev, unsigned int rq_depth,
>>>> }
>>>> }
>>>>
>>>> - if (hw_ioctxt.rx_buf_sz_idx == -1)
>>>> + if (hw_ioctxt.rx_buf_sz_idx == (uint16_t)-1)
>>>> return -EINVAL;
>>>>
>>>> hw_ioctxt.sq_depth = ilog2(sq_depth);
>>>>
>>>
>>> Many thanks, Colin.
>>> I prefer to avoid casting when possible, what do you think about replacing the condition by:
>>>
>>> if (rx_buf_sz_table[i].sz != HINIC_RX_BUF_SZ)
>>> return -EINVAL;
>>>
>>
>> Does that work as expected when rx_buf_sz_table[i].sz == -1?
>
> No it doesn't. Please, don't ask rhetorical questions. I have a
> toddler and I constantly ask him toddler level questions and it drives
> me nuts that all the adults in the room will answer me... "Yes, I
> already know that's a cow. I was quizing my son. But thank you!"
> Meanwhile I can't resist answering questions myself...
>
> The code looks like this:
>
> drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
> 345 hw_ioctxt.rq_depth = ilog2(rq_depth);
> 346
> 347 for (i = 0; ; i++) {
> 348 if ((rx_buf_sz_table[i].sz == HINIC_RX_BUF_SZ) ||
> 349 (rx_buf_sz_table[i].sz == -1)) {
> 350 hw_ioctxt.rx_buf_sz_idx = rx_buf_sz_table[i].idx;
> 351 break;
> 352 }
> 353 }
> 354
> 355 if (hw_ioctxt.rx_buf_sz_idx == -1)
> 356 return -EINVAL;
> 357
>
> The loop doesn't make sense. We are looping through rx_buf_sz_table[]
> until we hit 2048 or -1. But 2048 comes first so we always get there
> and break.
>
> We may as well replace all that code with:
>
> hw_ioctxt.rx_buf_sz_idx = 11;
>
> Something is very wrong.
>
> regards,
> dan carpenter
>
>
> .
>
Hi Dan,
What if HINIC_RX_BUF_SZ is changed to another value?
The test checks if the HINIC_RX_BUF_SZ is in the table, if not return -EINVAL.
Therefore I think the check of rx_buf_sz_table[i].sz != HINIC_RX_BUF_SZ is better.
Aviad
^ permalink raw reply
* Re: [PATCH][next] net: hinic: fix comparison of a uint16_t type with -1
From: Dan Carpenter @ 2017-08-24 9:29 UTC (permalink / raw)
To: Colin Ian King; +Cc: Aviad Krawczyk, netdev, kernel-janitors, linux-kernel
In-Reply-To: <d37c85d8-3954-e7b7-e569-f18c2b062966@canonical.com>
On Thu, Aug 24, 2017 at 09:54:03AM +0100, Colin Ian King wrote:
> On 24/08/17 09:48, Aviad Krawczyk wrote:
> > On 8/23/2017 6:39 PM, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> The comparison of hw_ioctxt.rx_buf_sz_idx == -1 is always false because
> >> rx_buf_sz_idx is a uint16_t. Fix this by explicitly casting -1 to uint16_t.
> >>
> >> Detected by CoverityScan, CID#1454559 ("Operands don't affect result")
> >>
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >> drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
> >> index 09dec6de8dd5..71e26070fb7f 100644
> >> --- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
> >> +++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
> >> @@ -352,7 +352,7 @@ static int set_hw_ioctxt(struct hinic_hwdev *hwdev, unsigned int rq_depth,
> >> }
> >> }
> >>
> >> - if (hw_ioctxt.rx_buf_sz_idx == -1)
> >> + if (hw_ioctxt.rx_buf_sz_idx == (uint16_t)-1)
> >> return -EINVAL;
> >>
> >> hw_ioctxt.sq_depth = ilog2(sq_depth);
> >>
> >
> > Many thanks, Colin.
> > I prefer to avoid casting when possible, what do you think about replacing the condition by:
> >
> > if (rx_buf_sz_table[i].sz != HINIC_RX_BUF_SZ)
> > return -EINVAL;
> >
>
> Does that work as expected when rx_buf_sz_table[i].sz == -1?
No it doesn't. Please, don't ask rhetorical questions. I have a
toddler and I constantly ask him toddler level questions and it drives
me nuts that all the adults in the room will answer me... "Yes, I
already know that's a cow. I was quizing my son. But thank you!"
Meanwhile I can't resist answering questions myself...
The code looks like this:
drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
345 hw_ioctxt.rq_depth = ilog2(rq_depth);
346
347 for (i = 0; ; i++) {
348 if ((rx_buf_sz_table[i].sz == HINIC_RX_BUF_SZ) ||
349 (rx_buf_sz_table[i].sz == -1)) {
350 hw_ioctxt.rx_buf_sz_idx = rx_buf_sz_table[i].idx;
351 break;
352 }
353 }
354
355 if (hw_ioctxt.rx_buf_sz_idx == -1)
356 return -EINVAL;
357
The loop doesn't make sense. We are looping through rx_buf_sz_table[]
until we hit 2048 or -1. But 2048 comes first so we always get there
and break.
We may as well replace all that code with:
hw_ioctxt.rx_buf_sz_idx = 11;
Something is very wrong.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] net/mlx4_core: make mlx4_profile const
From: Tariq Toukan @ 2017-08-24 9:04 UTC (permalink / raw)
To: Bhumika Goyal, julia.lawall, tariqt, netdev, linux-rdma,
linux-kernel
In-Reply-To: <1503492459-3110-1-git-send-email-bhumirks@gmail.com>
On 23/08/2017 3:47 PM, Bhumika Goyal wrote:
> Make these const as they are only used in a copy operation.
>
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
> index 541ce9e..43fb2eb 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -121,7 +121,7 @@
> DRV_NAME ": Mellanox ConnectX core driver v"
> DRV_VERSION "\n";
>
> -static struct mlx4_profile default_profile = {
> +static const struct mlx4_profile default_profile = {
> .num_qp = 1 << 18,
> .num_srq = 1 << 16,
> .rdmarc_per_qp = 1 << 4,
> @@ -131,7 +131,7 @@
> .num_mtt = 1 << 20, /* It is really num mtt segements */
> };
>
> -static struct mlx4_profile low_mem_profile = {
> +static const struct mlx4_profile low_mem_profile = {
> .num_qp = 1 << 17,
> .num_srq = 1 << 6,
> .rdmarc_per_qp = 1 << 4,
>
Thanks Bhumika.
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
^ permalink raw reply
* Re: [PATCH][next] net: hinic: fix comparison of a uint16_t type with -1
From: Colin Ian King @ 2017-08-24 8:54 UTC (permalink / raw)
To: Aviad Krawczyk, netdev; +Cc: kernel-janitors, linux-kernel
In-Reply-To: <58f312cc-5f24-83b6-d12d-c66e4fd4dad2@huawei.com>
On 24/08/17 09:48, Aviad Krawczyk wrote:
> On 8/23/2017 6:39 PM, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The comparison of hw_ioctxt.rx_buf_sz_idx == -1 is always false because
>> rx_buf_sz_idx is a uint16_t. Fix this by explicitly casting -1 to uint16_t.
>>
>> Detected by CoverityScan, CID#1454559 ("Operands don't affect result")
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
>> index 09dec6de8dd5..71e26070fb7f 100644
>> --- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
>> +++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
>> @@ -352,7 +352,7 @@ static int set_hw_ioctxt(struct hinic_hwdev *hwdev, unsigned int rq_depth,
>> }
>> }
>>
>> - if (hw_ioctxt.rx_buf_sz_idx == -1)
>> + if (hw_ioctxt.rx_buf_sz_idx == (uint16_t)-1)
>> return -EINVAL;
>>
>> hw_ioctxt.sq_depth = ilog2(sq_depth);
>>
>
> Many thanks, Colin.
> I prefer to avoid casting when possible, what do you think about replacing the condition by:
>
> if (rx_buf_sz_table[i].sz != HINIC_RX_BUF_SZ)
> return -EINVAL;
>
Does that work as expected when rx_buf_sz_table[i].sz == -1?
^ permalink raw reply
* Re: [PATCH][next] net: hinic: fix comparison of a uint16_t type with -1
From: Aviad Krawczyk @ 2017-08-24 8:48 UTC (permalink / raw)
To: Colin King, netdev; +Cc: kernel-janitors, linux-kernel
In-Reply-To: <20170823153936.22857-1-colin.king@canonical.com>
On 8/23/2017 6:39 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The comparison of hw_ioctxt.rx_buf_sz_idx == -1 is always false because
> rx_buf_sz_idx is a uint16_t. Fix this by explicitly casting -1 to uint16_t.
>
> Detected by CoverityScan, CID#1454559 ("Operands don't affect result")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
> index 09dec6de8dd5..71e26070fb7f 100644
> --- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
> +++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
> @@ -352,7 +352,7 @@ static int set_hw_ioctxt(struct hinic_hwdev *hwdev, unsigned int rq_depth,
> }
> }
>
> - if (hw_ioctxt.rx_buf_sz_idx == -1)
> + if (hw_ioctxt.rx_buf_sz_idx == (uint16_t)-1)
> return -EINVAL;
>
> hw_ioctxt.sq_depth = ilog2(sq_depth);
>
Many thanks, Colin.
I prefer to avoid casting when possible, what do you think about replacing the condition by:
if (rx_buf_sz_table[i].sz != HINIC_RX_BUF_SZ)
return -EINVAL;
^ permalink raw reply
* [PATCH net-next 07/13] net: mvpp2: improve the link management function
From: Antoine Tenart @ 2017-08-24 8:38 UTC (permalink / raw)
To: davem, kishon, andrew, jason, sebastian.hesselbarth,
gregory.clement
Cc: Antoine Tenart, thomas.petazzoni, nadavh, linux, linux-kernel, mw,
stefanc, miquel.raynal, netdev
In-Reply-To: <20170824083823.16826-1-antoine.tenart@free-electrons.com>
When the link status changes, the phylib calls the link_event function
in the mvpp2 driver. Before this patch only the egress/ingress transmit
was enabled/disabled. This patch adds more functionality to the link
status management code by enabling/disabling the port per-cpu
interrupts, and the port itself. The queues are now stopped as well, and
the netif carrier helpers are called.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
drivers/net/ethernet/marvell/mvpp2.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index ebcc89b8f792..99847fec1c5a 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -5753,14 +5753,24 @@ static void mvpp2_link_event(struct net_device *dev)
port->link = phydev->link;
if (phydev->link) {
+ mvpp2_interrupts_enable(port);
+ mvpp2_port_enable(port);
+
mvpp2_egress_enable(port);
mvpp2_ingress_enable(port);
+ netif_carrier_on(dev);
+ netif_tx_wake_all_queues(dev);
} else {
port->duplex = -1;
port->speed = 0;
+ netif_tx_stop_all_queues(dev);
+ netif_carrier_off(dev);
mvpp2_ingress_disable(port);
mvpp2_egress_disable(port);
+
+ mvpp2_port_disable(port);
+ mvpp2_interrupts_disable(port);
}
phy_print_status(phydev);
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 13/13] arm64: defconfig: enable Marvell CP110 comphy
From: Antoine Tenart @ 2017-08-24 8:38 UTC (permalink / raw)
To: davem, kishon, andrew, jason, sebastian.hesselbarth,
gregory.clement
Cc: Miquel Raynal, thomas.petazzoni, nadavh, linux, linux-kernel, mw,
stefanc, netdev, Antoine Tenart
In-Reply-To: <20170824083823.16826-1-antoine.tenart@free-electrons.com>
From: Miquel Raynal <miquel.raynal@free-electrons.com>
The comphy is an hardware block giving access to common PHYs that can be
used by various other engines (Network, SATA, ...). This is used on
Marvell 7k/8k platforms for now. Enable the corresponding driver.
Signed-off-by: Miquel Raynal <miquel.raynal@free-electrons.com>
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index cdde4f56a281..e671e37b30af 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -509,6 +509,7 @@ CONFIG_PWM_TEGRA=m
CONFIG_PHY_RCAR_GEN3_USB2=y
CONFIG_PHY_HI6220_USB=y
CONFIG_PHY_SUN4I_USB=y
+CONFIG_PHY_MVEBU_CP110_COMPHY=y
CONFIG_PHY_ROCKCHIP_INNO_USB2=y
CONFIG_PHY_ROCKCHIP_EMMC=y
CONFIG_PHY_ROCKCHIP_PCIE=m
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 12/13] arm64: dts: marvell: mcbin: add comphy references to Ethernet ports
From: Antoine Tenart @ 2017-08-24 8:38 UTC (permalink / raw)
To: davem, kishon, andrew, jason, sebastian.hesselbarth,
gregory.clement
Cc: Antoine Tenart, thomas.petazzoni, nadavh, linux, linux-kernel, mw,
stefanc, miquel.raynal, netdev
In-Reply-To: <20170824083823.16826-1-antoine.tenart@free-electrons.com>
This patch adds comphy phandles to the Ethernet ports in the mcbin
device tree. The comphy is used to configure the serdes PHYs used by
these ports.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
index 6cb4b000e1ac..dc4b6f3f25f6 100644
--- a/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
+++ b/arch/arm64/boot/dts/marvell/armada-8040-mcbin.dts
@@ -148,6 +148,7 @@
&cpm_eth0 {
status = "okay";
phy = <&phy0>;
+ phys = <&cpm_comphy4 0>;
phy-mode = "10gbase-kr";
};
@@ -181,6 +182,7 @@
&cps_eth0 {
status = "okay";
phy = <&phy1>;
+ phys = <&cps_comphy4 0>;
phy-mode = "10gbase-kr";
};
@@ -189,6 +191,7 @@
status = "okay";
phy = <&ge_phy>;
phy-mode = "sgmii";
+ phys = <&cps_comphy0 1>;
};
&cps_sata0 {
--
2.13.5
^ permalink raw reply related
* [PATCH net-next 11/13] arm64: dts: marvell: add comphy nodes on cp110 master and slave
From: Antoine Tenart @ 2017-08-24 8:38 UTC (permalink / raw)
To: davem, kishon, andrew, jason, sebastian.hesselbarth,
gregory.clement
Cc: Antoine Tenart, thomas.petazzoni, nadavh, linux, linux-kernel, mw,
stefanc, miquel.raynal, netdev
In-Reply-To: <20170824083823.16826-1-antoine.tenart@free-electrons.com>
Now that the comphy driver is available, this patch adds the
corresponding nodes in the cp110 master and slave device trees.
Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
.../boot/dts/marvell/armada-cp110-master.dtsi | 38 ++++++++++++++++++++++
.../arm64/boot/dts/marvell/armada-cp110-slave.dtsi | 38 ++++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
index 9b2581473183..f2a50552bad4 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-master.dtsi
@@ -91,6 +91,44 @@
};
};
+ cpm_comphy: phy@120000 {
+ compatible = "marvell,comphy-cp110";
+ reg = <0x120000 0x6000>;
+ marvell,system-controller = <&cpm_syscon0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpm_comphy0: phy@0 {
+ reg = <0>;
+ #phy-cells = <1>;
+ };
+
+ cpm_comphy1: phy@1 {
+ reg = <1>;
+ #phy-cells = <1>;
+ };
+
+ cpm_comphy2: phy@2 {
+ reg = <2>;
+ #phy-cells = <1>;
+ };
+
+ cpm_comphy3: phy@3 {
+ reg = <3>;
+ #phy-cells = <1>;
+ };
+
+ cpm_comphy4: phy@4 {
+ reg = <4>;
+ #phy-cells = <1>;
+ };
+
+ cpm_comphy5: phy@5 {
+ reg = <5>;
+ #phy-cells = <1>;
+ };
+ };
+
cpm_mdio: mdio@12a200 {
#address-cells = <1>;
#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
index d3902f218c46..bd7f7d0e6de9 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp110-slave.dtsi
@@ -98,6 +98,44 @@
};
};
+ cps_comphy: phy@120000 {
+ compatible = "marvell,comphy-cp110";
+ reg = <0x120000 0x6000>;
+ marvell,system-controller = <&cps_syscon0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cps_comphy0: phy@0 {
+ reg = <0>;
+ #phy-cells = <1>;
+ };
+
+ cps_comphy1: phy@1 {
+ reg = <1>;
+ #phy-cells = <1>;
+ };
+
+ cps_comphy2: phy@2 {
+ reg = <2>;
+ #phy-cells = <1>;
+ };
+
+ cps_comphy3: phy@3 {
+ reg = <3>;
+ #phy-cells = <1>;
+ };
+
+ cps_comphy4: phy@4 {
+ reg = <4>;
+ #phy-cells = <1>;
+ };
+
+ cps_comphy5: phy@5 {
+ reg = <5>;
+ #phy-cells = <1>;
+ };
+ };
+
cps_mdio: mdio@12a200 {
#address-cells = <1>;
#size-cells = <0>;
--
2.13.5
^ permalink raw reply related
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