* [PATCH net-next 03/15] selftests: Add ipv4 ping tests to fcnal-test
From: David Ahern @ 2019-08-01 18:56 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
In-Reply-To: <20190801185648.27653-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Add ping tests to fcnal-test.sh. Covers the permutations of directly
connected addresses, routed destinations, VRF and non-VRF, and expected
failures.
Setup includes unreachable routes and fib rules blocking traffic.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fcnal-test.sh | 268 +++++++++++++++++++++++++++++-
1 file changed, 267 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
index 22cfbd2fd09c..3f6e786b34ae 100755
--- a/tools/testing/selftests/net/fcnal-test.sh
+++ b/tools/testing/selftests/net/fcnal-test.sh
@@ -448,6 +448,270 @@ setup()
}
################################################################################
+# IPv4
+
+ipv4_ping_novrf()
+{
+ local a
+
+ #
+ # out
+ #
+ for a in ${NSB_IP} ${NSB_LO_IP}
+ do
+ log_start
+ run_cmd ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 0 "ping out"
+
+ log_start
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 0 "ping out, device bind"
+
+ log_start
+ run_cmd ping -c1 -w1 -I ${NSA_LO_IP} ${a}
+ log_test_addr ${a} $? 0 "ping out, address bind"
+ done
+
+ #
+ # in
+ #
+ for a in ${NSA_IP} ${NSA_LO_IP}
+ do
+ log_start
+ run_cmd_nsb ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 0 "ping in"
+ done
+
+ #
+ # local traffic
+ #
+ for a in ${NSA_IP} ${NSA_LO_IP} 127.0.0.1
+ do
+ log_start
+ run_cmd ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 0 "ping local"
+ done
+
+ #
+ # local traffic, socket bound to device
+ #
+ # address on device
+ a=${NSA_IP}
+ log_start
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 0 "ping local, device bind"
+
+ # loopback addresses not reachable from device bind
+ # fails in a really weird way though because ipv4 special cases
+ # route lookups with oif set.
+ for a in ${NSA_LO_IP} 127.0.0.1
+ do
+ log_start
+ show_hint "Fails since address on loopback device is out of device scope"
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 1 "ping local, device bind"
+ done
+
+ #
+ # ip rule blocks reachability to remote address
+ #
+ log_start
+ setup_cmd ip rule add pref 32765 from all lookup local
+ setup_cmd ip rule del pref 0 from all lookup local
+ setup_cmd ip rule add pref 50 to ${NSB_LO_IP} prohibit
+ setup_cmd ip rule add pref 51 from ${NSB_IP} prohibit
+
+ a=${NSB_LO_IP}
+ run_cmd ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 2 "ping out, blocked by rule"
+
+ # NOTE: ipv4 actually allows the lookup to fail and yet still create
+ # a viable rtable if the oif (e.g., bind to device) is set, so this
+ # case succeeds despite the rule
+ # run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+
+ a=${NSA_LO_IP}
+ log_start
+ show_hint "Response generates ICMP (or arp request is ignored) due to ip rule"
+ run_cmd_nsb ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 1 "ping in, blocked by rule"
+
+ [ "$VERBOSE" = "1" ] && echo
+ setup_cmd ip rule del pref 32765 from all lookup local
+ setup_cmd ip rule add pref 0 from all lookup local
+ setup_cmd ip rule del pref 50 to ${NSB_LO_IP} prohibit
+ setup_cmd ip rule del pref 51 from ${NSB_IP} prohibit
+
+ #
+ # route blocks reachability to remote address
+ #
+ log_start
+ setup_cmd ip route replace unreachable ${NSB_LO_IP}
+ setup_cmd ip route replace unreachable ${NSB_IP}
+
+ a=${NSB_LO_IP}
+ run_cmd ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 2 "ping out, blocked by route"
+
+ # NOTE: ipv4 actually allows the lookup to fail and yet still create
+ # a viable rtable if the oif (e.g., bind to device) is set, so this
+ # case succeeds despite not having a route for the address
+ # run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+
+ a=${NSA_LO_IP}
+ log_start
+ show_hint "Response is dropped (or arp request is ignored) due to ip route"
+ run_cmd_nsb ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 1 "ping in, blocked by route"
+
+ #
+ # remove 'remote' routes; fallback to default
+ #
+ log_start
+ setup_cmd ip ro del ${NSB_LO_IP}
+
+ a=${NSB_LO_IP}
+ run_cmd ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 2 "ping out, unreachable default route"
+
+ # NOTE: ipv4 actually allows the lookup to fail and yet still create
+ # a viable rtable if the oif (e.g., bind to device) is set, so this
+ # case succeeds despite not having a route for the address
+ # run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+}
+
+ipv4_ping_vrf()
+{
+ local a
+
+ # should default on; does not exist on older kernels
+ set_sysctl net.ipv4.raw_l3mdev_accept=1 2>/dev/null
+
+ #
+ # out
+ #
+ for a in ${NSB_IP} ${NSB_LO_IP}
+ do
+ log_start
+ run_cmd ping -c1 -w1 -I ${VRF} ${a}
+ log_test_addr ${a} $? 0 "ping out, VRF bind"
+
+ log_start
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 0 "ping out, device bind"
+
+ log_start
+ run_cmd ip vrf exec ${VRF} ping -c1 -w1 -I ${NSA_IP} ${a}
+ log_test_addr ${a} $? 0 "ping out, vrf device + dev address bind"
+
+ log_start
+ run_cmd ip vrf exec ${VRF} ping -c1 -w1 -I ${VRF_IP} ${a}
+ log_test_addr ${a} $? 0 "ping out, vrf device + vrf address bind"
+ done
+
+ #
+ # in
+ #
+ for a in ${NSA_IP} ${VRF_IP}
+ do
+ log_start
+ run_cmd_nsb ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 0 "ping in"
+ done
+
+ #
+ # local traffic, local address
+ #
+ for a in ${NSA_IP} ${VRF_IP} 127.0.0.1
+ do
+ log_start
+ show_hint "Source address should be ${a}"
+ run_cmd ping -c1 -w1 -I ${VRF} ${a}
+ log_test_addr ${a} $? 0 "ping local, VRF bind"
+ done
+
+ #
+ # local traffic, socket bound to device
+ #
+ # address on device
+ a=${NSA_IP}
+ log_start
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 0 "ping local, device bind"
+
+ # vrf device is out of scope
+ for a in ${VRF_IP} 127.0.0.1
+ do
+ log_start
+ show_hint "Fails since address on vrf device is out of device scope"
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 1 "ping local, device bind"
+ done
+
+ #
+ # ip rule blocks address
+ #
+ log_start
+ setup_cmd ip rule add pref 50 to ${NSB_LO_IP} prohibit
+ setup_cmd ip rule add pref 51 from ${NSB_IP} prohibit
+
+ a=${NSB_LO_IP}
+ run_cmd ping -c1 -w1 -I ${VRF} ${a}
+ log_test_addr ${a} $? 2 "ping out, vrf bind, blocked by rule"
+
+ log_start
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 2 "ping out, device bind, blocked by rule"
+
+ a=${NSA_LO_IP}
+ log_start
+ show_hint "Response lost due to ip rule"
+ run_cmd_nsb ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 1 "ping in, blocked by rule"
+
+ [ "$VERBOSE" = "1" ] && echo
+ setup_cmd ip rule del pref 50 to ${NSB_LO_IP} prohibit
+ setup_cmd ip rule del pref 51 from ${NSB_IP} prohibit
+
+ #
+ # remove 'remote' routes; fallback to default
+ #
+ log_start
+ setup_cmd ip ro del vrf ${VRF} ${NSB_LO_IP}
+
+ a=${NSB_LO_IP}
+ run_cmd ping -c1 -w1 -I ${VRF} ${a}
+ log_test_addr ${a} $? 2 "ping out, vrf bind, unreachable route"
+
+ log_start
+ run_cmd ping -c1 -w1 -I ${NSA_DEV} ${a}
+ log_test_addr ${a} $? 2 "ping out, device bind, unreachable route"
+
+ a=${NSA_LO_IP}
+ log_start
+ show_hint "Response lost by unreachable route"
+ run_cmd_nsb ping -c1 -w1 ${a}
+ log_test_addr ${a} $? 1 "ping in, unreachable route"
+}
+
+ipv4_ping()
+{
+ log_section "IPv4 ping"
+
+ log_subsection "No VRF"
+ setup
+ set_sysctl net.ipv4.raw_l3mdev_accept=0 2>/dev/null
+ ipv4_ping_novrf
+ setup
+ set_sysctl net.ipv4.raw_l3mdev_accept=1 2>/dev/null
+ ipv4_ping_novrf
+
+ log_subsection "With VRF"
+ setup "yes"
+ ipv4_ping_vrf
+}
+
+################################################################################
# usage
usage()
@@ -467,7 +731,7 @@ EOF
################################################################################
# main
-TESTS_IPV4=""
+TESTS_IPV4="ipv4_ping"
TESTS_IPV6=""
PAUSE_ON_FAIL=no
PAUSE=no
@@ -506,6 +770,8 @@ declare -i nsuccess=0
for t in $TESTS
do
case $t in
+ ipv4_ping|ping) ipv4_ping;;
+
# setup namespaces and config, but do not run any tests
setup) setup; exit 0;;
vrf_setup) setup "yes"; exit 0;;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 00/15] net: Add functional tests for L3 and L4
From: David Ahern @ 2019-08-01 18:56 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
From: David Ahern <dsahern@gmail.com>
This is a port the functional test cases created during the development
of the VRF feature. It covers various permutations of icmp, tcp and udp
for IPv4 and IPv6 including negative tests.
David Ahern (15):
selftests: Add nettest
selftests: Setup for functional tests for fib and socket lookups
selftests: Add ipv4 ping tests to fcnal-test
selftests: Add ipv6 ping tests to fcnal-test
selftests: Add ipv4 tcp tests to fcnal-test
selftests: Add ipv6 tcp tests to fcnal-test
selftests: Add ipv4 udp tests to fcnal-test
selftests: Add ipv6 udp tests to fcnal-test
selftests: Add ipv4 address bind tests to fcnal-test
selftests: Add ipv6 address bind tests to fcnal-test
selftests: Add ipv4 runtime tests to fcnal-test
selftests: Add ipv6 runtime tests to fcnal-test
selftests: Add ipv4 netfilter tests to fcnal-test
selftests: Add ipv6 netfilter tests to fcnal-test
selftests: Add use case section to fcnal-test
tools/testing/selftests/net/Makefile | 4 +-
tools/testing/selftests/net/fcnal-test.sh | 3458 +++++++++++++++++++++++++++++
tools/testing/selftests/net/nettest.c | 1756 +++++++++++++++
3 files changed, 5216 insertions(+), 2 deletions(-)
create mode 100755 tools/testing/selftests/net/fcnal-test.sh
create mode 100644 tools/testing/selftests/net/nettest.c
--
2.11.0
^ permalink raw reply
* [PATCH net-next 1/5] net: dsa: mv88e6xxx: lock mutex in vlan_prepare
From: Vivien Didelot @ 2019-08-01 18:36 UTC (permalink / raw)
Cc: linux-kernel, Vivien Didelot, Rasmus Villemoes, f.fainelli,
andrew, netdev, davem
In-Reply-To: <20190801183637.24841-1-vivien.didelot@gmail.com>
Lock the mutex in the mv88e6xxx_port_vlan_prepare function
called by the DSA stack, instead of doing it in the internal
mv88e6xxx_port_check_hw_vlan helper.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 2e500428670c..1b2cb46d3b53 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1453,12 +1453,10 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
if (!vid_begin)
return -EOPNOTSUPP;
- mv88e6xxx_reg_lock(chip);
-
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
- goto unlock;
+ return err;
if (!vlan.valid)
break;
@@ -1487,15 +1485,11 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
dev_err(ds->dev, "p%d: hw VLAN %d already used by port %d in %s\n",
port, vlan.vid, i,
netdev_name(dsa_to_port(ds, i)->bridge_dev));
- err = -EOPNOTSUPP;
- goto unlock;
+ return -EOPNOTSUPP;
}
} while (vlan.vid < vid_end);
-unlock:
- mv88e6xxx_reg_unlock(chip);
-
- return err;
+ return 0;
}
static int mv88e6xxx_port_vlan_filtering(struct dsa_switch *ds, int port,
@@ -1529,15 +1523,15 @@ mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
/* If the requested port doesn't belong to the same bridge as the VLAN
* members, do not support it (yet) and fallback to software VLAN.
*/
+ mv88e6xxx_reg_lock(chip);
err = mv88e6xxx_port_check_hw_vlan(ds, port, vlan->vid_begin,
vlan->vid_end);
- if (err)
- return err;
+ mv88e6xxx_reg_unlock(chip);
/* We don't need any dynamic resource from the kernel (yet),
* so skip the prepare phase.
*/
- return 0;
+ return err;
}
static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
--
2.22.0
^ permalink raw reply related
* [PATCH net-next 2/5] net: dsa: mv88e6xxx: explicit entry passed to vtu_getnext
From: Vivien Didelot @ 2019-08-01 18:36 UTC (permalink / raw)
Cc: linux-kernel, Vivien Didelot, Rasmus Villemoes, f.fainelli,
andrew, netdev, davem
In-Reply-To: <20190801183637.24841-1-vivien.didelot@gmail.com>
mv88e6xxx_vtu_getnext interprets two members from the input
mv88e6xxx_vtu_entry structure: the (excluded) vid member to start
the iteration from, and the valid argument specifying whether the VID
must be written or not (only required once at the start of a loop).
Explicit the assignation of these two fields right before calling
mv88e6xxx_vtu_getnext, as it is done in the mv88e6xxx_vtu_get wrapper.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 1b2cb46d3b53..c825fa3477fa 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1361,9 +1361,7 @@ static int mv88e6xxx_vtu_loadpurge(struct mv88e6xxx_chip *chip,
static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
{
DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
- struct mv88e6xxx_vtu_entry vlan = {
- .vid = chip->info->max_vid,
- };
+ struct mv88e6xxx_vtu_entry vlan;
int i, err;
bitmap_zero(fid_bitmap, MV88E6XXX_N_FID);
@@ -1378,6 +1376,9 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
}
/* Set every FID bit used by the VLAN entries */
+ vlan.vid = chip->info->max_vid;
+ vlan.valid = false;
+
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
@@ -1441,9 +1442,7 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
u16 vid_begin, u16 vid_end)
{
struct mv88e6xxx_chip *chip = ds->priv;
- struct mv88e6xxx_vtu_entry vlan = {
- .vid = vid_begin - 1,
- };
+ struct mv88e6xxx_vtu_entry vlan;
int i, err;
/* DSA and CPU ports have to be members of multiple vlans */
@@ -1453,6 +1452,9 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
if (!vid_begin)
return -EOPNOTSUPP;
+ vlan.vid = vid_begin - 1;
+ vlan.valid = false;
+
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
@@ -1789,9 +1791,7 @@ static int mv88e6xxx_port_db_dump_fid(struct mv88e6xxx_chip *chip,
static int mv88e6xxx_port_db_dump(struct mv88e6xxx_chip *chip, int port,
dsa_fdb_dump_cb_t *cb, void *data)
{
- struct mv88e6xxx_vtu_entry vlan = {
- .vid = chip->info->max_vid,
- };
+ struct mv88e6xxx_vtu_entry vlan;
u16 fid;
int err;
@@ -1805,6 +1805,9 @@ static int mv88e6xxx_port_db_dump(struct mv88e6xxx_chip *chip, int port,
return err;
/* Dump VLANs' Filtering Information Databases */
+ vlan.vid = chip->info->max_vid;
+ vlan.valid = false;
+
do {
err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
--
2.22.0
^ permalink raw reply related
* [PATCH net-next 3/5] net: dsa: mv88e6xxx: call vtu_getnext directly in db load/purge
From: Vivien Didelot @ 2019-08-01 18:36 UTC (permalink / raw)
Cc: linux-kernel, Vivien Didelot, Rasmus Villemoes, f.fainelli,
andrew, netdev, davem
In-Reply-To: <20190801183637.24841-1-vivien.didelot@gmail.com>
mv88e6xxx_vtu_getnext is simple enough to call it directly in the
mv88e6xxx_port_db_load_purge function and explicit the return code
expected by switchdev for software VLANs when an hardware VLAN does
not exist.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c825fa3477fa..42ab57dbc790 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1540,23 +1540,36 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
const unsigned char *addr, u16 vid,
u8 state)
{
- struct mv88e6xxx_vtu_entry vlan;
struct mv88e6xxx_atu_entry entry;
+ struct mv88e6xxx_vtu_entry vlan;
+ u16 fid;
int err;
/* Null VLAN ID corresponds to the port private database */
- if (vid == 0)
- err = mv88e6xxx_port_get_fid(chip, port, &vlan.fid);
- else
- err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
- if (err)
- return err;
+ if (vid == 0) {
+ err = mv88e6xxx_port_get_fid(chip, port, &fid);
+ if (err)
+ return err;
+ } else {
+ vlan.vid = vid - 1;
+ vlan.valid = false;
+
+ err = mv88e6xxx_vtu_getnext(chip, &vlan);
+ if (err)
+ return err;
+
+ /* switchdev expects -EOPNOTSUPP to honor software VLANs */
+ if (vlan.vid != vid || !vlan.valid)
+ return -EOPNOTSUPP;
+
+ fid = vlan.fid;
+ }
entry.state = MV88E6XXX_G1_ATU_DATA_STATE_UNUSED;
ether_addr_copy(entry.mac, addr);
eth_addr_dec(entry.mac);
- err = mv88e6xxx_g1_atu_getnext(chip, vlan.fid, &entry);
+ err = mv88e6xxx_g1_atu_getnext(chip, fid, &entry);
if (err)
return err;
@@ -1577,7 +1590,7 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
entry.state = state;
}
- return mv88e6xxx_g1_atu_loadpurge(chip, vlan.fid, &entry);
+ return mv88e6xxx_g1_atu_loadpurge(chip, fid, &entry);
}
static int mv88e6xxx_port_add_broadcast(struct mv88e6xxx_chip *chip, int port,
--
2.22.0
^ permalink raw reply related
* [PATCH net-next 4/5] net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_del
From: Vivien Didelot @ 2019-08-01 18:36 UTC (permalink / raw)
Cc: linux-kernel, Vivien Didelot, Rasmus Villemoes, f.fainelli,
andrew, netdev, davem
In-Reply-To: <20190801183637.24841-1-vivien.didelot@gmail.com>
Wrapping mv88e6xxx_vtu_getnext makes the code less easy to read.
Explicit the call to mv88e6xxx_vtu_getnext in _mv88e6xxx_port_vlan_del
and the return value expected by switchdev in case of software VLANs.
At the same time, rename the helper using an old underscore convention.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 42ab57dbc790..50a6dbcc669c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1671,18 +1671,27 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
mv88e6xxx_reg_unlock(chip);
}
-static int _mv88e6xxx_port_vlan_del(struct mv88e6xxx_chip *chip,
- int port, u16 vid)
+static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
+ int port, u16 vid)
{
struct mv88e6xxx_vtu_entry vlan;
int i, err;
- err = mv88e6xxx_vtu_get(chip, vid, &vlan, false);
+ if (!vid)
+ return -EOPNOTSUPP;
+
+ vlan.vid = vid - 1;
+ vlan.valid = false;
+
+ err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
return err;
- /* Tell switchdev if this VLAN is handled in software */
- if (vlan.member[port] == MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
+ /* If the VLAN doesn't exist in hardware or the port isn't a member,
+ * tell switchdev that this VLAN is likely handled in software.
+ */
+ if (vlan.vid != vid || !vlan.valid ||
+ vlan.member[port] == MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
return -EOPNOTSUPP;
vlan.member[port] = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER;
@@ -1721,7 +1730,7 @@ static int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port,
goto unlock;
for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid) {
- err = _mv88e6xxx_port_vlan_del(chip, port, vid);
+ err = mv88e6xxx_port_vlan_leave(chip, port, vid);
if (err)
goto unlock;
--
2.22.0
^ permalink raw reply related
* [PATCH net-next 5/5] net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_add
From: Vivien Didelot @ 2019-08-01 18:36 UTC (permalink / raw)
Cc: linux-kernel, Vivien Didelot, Rasmus Villemoes, f.fainelli,
andrew, netdev, davem
In-Reply-To: <20190801183637.24841-1-vivien.didelot@gmail.com>
Wrapping mv88e6xxx_vtu_getnext makes the code less easy to read and
_mv88e6xxx_port_vlan_add is the only function requiring the preparation
of a new VLAN entry.
To simplify things up, remove the mv88e6xxx_vtu_get wrapper and
explicit the VLAN lookup in _mv88e6xxx_port_vlan_add. This rework
also avoids programming the broadcast entries again when changing a
port's membership, e.g. from tagged to untagged.
At the same time, rename the helper using an old underscore convention.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 93 +++++++++++++++-----------------
1 file changed, 44 insertions(+), 49 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 50a6dbcc669c..8c4216e7a4bb 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1401,43 +1401,6 @@ static int mv88e6xxx_atu_new(struct mv88e6xxx_chip *chip, u16 *fid)
return mv88e6xxx_g1_atu_flush(chip, *fid, true);
}
-static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
- struct mv88e6xxx_vtu_entry *entry, bool new)
-{
- int err;
-
- if (!vid)
- return -EOPNOTSUPP;
-
- entry->vid = vid - 1;
- entry->valid = false;
-
- err = mv88e6xxx_vtu_getnext(chip, entry);
- if (err)
- return err;
-
- if (entry->vid == vid && entry->valid)
- return 0;
-
- if (new) {
- int i;
-
- /* Initialize a fresh VLAN entry */
- memset(entry, 0, sizeof(*entry));
- entry->vid = vid;
-
- /* Exclude all ports */
- for (i = 0; i < mv88e6xxx_num_ports(chip); ++i)
- entry->member[i] =
- MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER;
-
- return mv88e6xxx_atu_new(chip, &entry->fid);
- }
-
- /* switchdev expects -EOPNOTSUPP to honor software VLANs */
- return -EOPNOTSUPP;
-}
-
static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
u16 vid_begin, u16 vid_end)
{
@@ -1616,26 +1579,58 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
return 0;
}
-static int _mv88e6xxx_port_vlan_add(struct mv88e6xxx_chip *chip, int port,
+static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port,
u16 vid, u8 member)
{
+ const u8 non_member = MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER;
struct mv88e6xxx_vtu_entry vlan;
- int err;
+ int i, err;
- err = mv88e6xxx_vtu_get(chip, vid, &vlan, true);
+ if (!vid)
+ return -EOPNOTSUPP;
+
+ vlan.vid = vid - 1;
+ vlan.valid = false;
+
+ err = mv88e6xxx_vtu_getnext(chip, &vlan);
if (err)
return err;
- if (vlan.valid && vlan.member[port] == member)
- return 0;
- vlan.valid = true;
- vlan.member[port] = member;
+ if (vlan.vid != vid || !vlan.valid) {
+ memset(&vlan, 0, sizeof(vlan));
- err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
- if (err)
- return err;
+ err = mv88e6xxx_atu_new(chip, &vlan.fid);
+ if (err)
+ return err;
- return mv88e6xxx_broadcast_setup(chip, vid);
+ for (i = 0; i < mv88e6xxx_num_ports(chip); ++i)
+ if (i == port)
+ vlan.member[i] = member;
+ else
+ vlan.member[i] = non_member;
+
+ vlan.vid = vid;
+ vlan.valid = true;
+
+ err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
+ if (err)
+ return err;
+
+ err = mv88e6xxx_broadcast_setup(chip, vlan.vid);
+ if (err)
+ return err;
+ } else if (vlan.member[port] != member) {
+ vlan.member[port] = member;
+
+ err = mv88e6xxx_vtu_loadpurge(chip, &vlan);
+ if (err)
+ return err;
+ } else {
+ dev_info(chip->dev, "p%d: already a member of VLAN %d\n",
+ port, vid);
+ }
+
+ return 0;
}
static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
@@ -1660,7 +1655,7 @@ static void mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port,
mv88e6xxx_reg_lock(chip);
for (vid = vlan->vid_begin; vid <= vlan->vid_end; ++vid)
- if (_mv88e6xxx_port_vlan_add(chip, port, vid, member))
+ if (mv88e6xxx_port_vlan_join(chip, port, vid, member))
dev_err(ds->dev, "p%d: failed to add VLAN %d%c\n", port,
vid, untagged ? 'u' : 't');
--
2.22.0
^ permalink raw reply related
* [PATCH net-next 0/5] net: dsa: mv88e6xxx: avoid some redundant VTU operations
From: Vivien Didelot @ 2019-08-01 18:36 UTC (permalink / raw)
Cc: linux-kernel, Vivien Didelot, Rasmus Villemoes, f.fainelli,
andrew, netdev, davem
The mv88e6xxx driver currently uses a mv88e6xxx_vtu_get wrapper to get a
single entry and uses a boolean to eventually initialize a fresh one.
However the fresh entry is only needed in one place and mv88e6xxx_vtu_getnext
is simple enough to call it directly. Doing so makes the code easier to read,
especially for the return code expected by switchdev to honor software VLANs.
In addition to not loading the VTU again when an entry is already correctly
programmed, this also allows to avoid programming the broadcast entries
again when updating a port's membership, from e.g. tagged to untagged.
This patch series removes the mv88e6xxx_vtu_get wrapper in favor of direct
calls to mv88e6xxx_vtu_getnext, and also renames the _mv88e6xxx_port_vlan_add
and _mv88e6xxx_port_vlan_del helpers using an old underscore prefix convention.
In case the port's membership is already correctly programmed in hardware,
the following debug message may be printed:
[ 745.989884] mv88e6085 2188000.ethernet-1:00: p4: already a member of VLAN 42
Vivien Didelot (5):
net: dsa: mv88e6xxx: lock mutex in vlan_prepare
net: dsa: mv88e6xxx: explicit entry passed to vtu_getnext
net: dsa: mv88e6xxx: call vtu_getnext directly in db load/purge
net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_del
net: dsa: mv88e6xxx: call vtu_getnext directly in vlan_add
drivers/net/dsa/mv88e6xxx/chip.c | 182 +++++++++++++++++--------------
1 file changed, 98 insertions(+), 84 deletions(-)
--
2.22.0
^ permalink raw reply
* Re: [PATCH mlx5-next 00/11] Mellanox, mlx5-next updates 2019-07-29
From: Saeed Mahameed @ 2019-08-01 18:23 UTC (permalink / raw)
To: netdev@vger.kernel.org, Leon Romanovsky,
linux-rdma@vger.kernel.org
In-Reply-To: <20190729211209.14772-1-saeedm@mellanox.com>
On Mon, 2019-07-29 at 21:12 +0000, Saeed Mahameed wrote:
> Hi All,
>
> This series include misc updates form mlx5 core driver.
>
> 1) Eli improves the handling of the support for QoS element type
> 2) Gavi refactors and prepares mlx5 flow counters for bluk allocation
> support
> 3) Parav, refactors and improves eswitch load/unload flows
> 4) Saeed, two misc cleanups
>
Applied to mlx5-next.
Thanks,
Saeed.
^ permalink raw reply
* Re: [PATCH mlx5-next 00/11] Mellanox, mlx5-next updates 2019-07-29
From: Saeed Mahameed @ 2019-08-01 18:23 UTC (permalink / raw)
To: netdev@vger.kernel.org, Leon Romanovsky,
linux-rdma@vger.kernel.org
In-Reply-To: <20190729211209.14772-1-saeedm@mellanox.com>
On Mon, 2019-07-29 at 21:12 +0000, Saeed Mahameed wrote:
> Hi All,
>
> This series include misc updates form mlx5 core driver.
>
> 1) Eli improves the handling of the support for QoS element type
> 2) Gavi refactors and prepares mlx5 flow counters for bluk allocation
> support
> 3) Parav, refactors and improves eswitch load/unload flows
> 4) Saeed, two misc cleanups
>
Applied to mlx5-next.
Thanks,
Saeed.
^ permalink raw reply
* [net v1 PATCH 4/4] net: fix bpf_xdp_adjust_head regression for generic-XDP
From: Jesper Dangaard Brouer @ 2019-08-01 18:00 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: xdp-newbies, Daniel Borkmann, brandon.cazander,
Alexei Starovoitov, Jesper Dangaard Brouer
In-Reply-To: <156468229108.27559.2443904494495785131.stgit@firesoul>
When generic-XDP was moved to a later processing step by commit
458bf2f224f0 ("net: core: support XDP generic on stacked devices.")
a regression was introduced when using bpf_xdp_adjust_head.
The issue is that after this commit the skb->network_header is now
changed prior to calling generic XDP and not after. Thus, if the header
is changed by XDP (via bpf_xdp_adjust_head), then skb->network_header
also need to be updated again. Fix by calling skb_reset_network_header().
Fixes: 458bf2f224f0 ("net: core: support XDP generic on stacked devices.")
Reported-by: Brandon Cazander <brandon.cazander@multapplied.net>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/dev.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index fc676b2610e3..b5533795c3c1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4374,12 +4374,17 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
act = bpf_prog_run_xdp(xdp_prog, xdp);
+ /* check if bpf_xdp_adjust_head was used */
off = xdp->data - orig_data;
- if (off > 0)
- __skb_pull(skb, off);
- else if (off < 0)
- __skb_push(skb, -off);
- skb->mac_header += off;
+ if (off) {
+ if (off > 0)
+ __skb_pull(skb, off);
+ else if (off < 0)
+ __skb_push(skb, -off);
+
+ skb->mac_header += off;
+ skb_reset_network_header(skb);
+ }
/* check if bpf_xdp_adjust_tail was used. it can only "shrink"
* pckt.
^ permalink raw reply related
* [net v1 PATCH 3/4] selftests/bpf: reduce time to execute test_xdp_vlan.sh
From: Jesper Dangaard Brouer @ 2019-08-01 18:00 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: xdp-newbies, Daniel Borkmann, brandon.cazander,
Alexei Starovoitov, Jesper Dangaard Brouer
In-Reply-To: <156468229108.27559.2443904494495785131.stgit@firesoul>
Given the increasing number of BPF selftests, it makes sense to
reduce the time to execute these tests. The ping parameters are
adjusted to reduce the time from measures 9 sec to approx 2.8 sec.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
tools/testing/selftests/bpf/test_xdp_vlan.sh | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_xdp_vlan.sh b/tools/testing/selftests/bpf/test_xdp_vlan.sh
index 7348661be815..bb8b0da91686 100755
--- a/tools/testing/selftests/bpf/test_xdp_vlan.sh
+++ b/tools/testing/selftests/bpf/test_xdp_vlan.sh
@@ -188,7 +188,7 @@ ip netns exec ns2 ip link set lo up
# At this point, the hosts cannot reach each-other,
# because ns2 are using VLAN tags on the packets.
-ip netns exec ns2 sh -c 'ping -W 1 -c 1 100.64.41.1 || echo "Okay ping fails"'
+ip netns exec ns2 sh -c 'ping -W 1 -c 1 100.64.41.1 || echo "Success: First ping must fail"'
# Now we can use the test_xdp_vlan.c program to pop/push these VLAN tags
@@ -210,8 +210,8 @@ ip netns exec ns1 tc filter add dev $DEVNS1 egress \
prio 1 handle 1 bpf da obj $FILE sec tc_vlan_push
# Now the namespaces can reach each-other, test with ping:
-ip netns exec ns2 ping -W 2 -c 3 $IPADDR1
-ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
+ip netns exec ns2 ping -i 0.2 -W 2 -c 2 $IPADDR1
+ip netns exec ns1 ping -i 0.2 -W 2 -c 2 $IPADDR2
# Second test: Replace xdp prog, that fully remove vlan header
#
@@ -224,5 +224,5 @@ ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE off
ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE object $FILE section $XDP_PROG
# Now the namespaces should still be able reach each-other, test with ping:
-ip netns exec ns2 ping -W 2 -c 3 $IPADDR1
-ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
+ip netns exec ns2 ping -i 0.2 -W 2 -c 2 $IPADDR1
+ip netns exec ns1 ping -i 0.2 -W 2 -c 2 $IPADDR2
^ permalink raw reply related
* [net v1 PATCH 2/4] selftests/bpf: add wrapper scripts for test_xdp_vlan.sh
From: Jesper Dangaard Brouer @ 2019-08-01 18:00 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: xdp-newbies, Daniel Borkmann, brandon.cazander,
Alexei Starovoitov, Jesper Dangaard Brouer
In-Reply-To: <156468229108.27559.2443904494495785131.stgit@firesoul>
In-order to test both native-XDP (xdpdrv) and generic-XDP (xdpgeneric)
create two wrapper test scripts, that start the test_xdp_vlan.sh script
with these modes.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
tools/testing/selftests/bpf/Makefile | 3 ++-
tools/testing/selftests/bpf/test_xdp_vlan.sh | 5 ++++-
.../selftests/bpf/test_xdp_vlan_mode_generic.sh | 9 +++++++++
.../selftests/bpf/test_xdp_vlan_mode_native.sh | 9 +++++++++
4 files changed, 24 insertions(+), 2 deletions(-)
create mode 100755 tools/testing/selftests/bpf/test_xdp_vlan_mode_generic.sh
create mode 100755 tools/testing/selftests/bpf/test_xdp_vlan_mode_native.sh
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 3bd0f4a0336a..29001f944db7 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -57,7 +57,8 @@ TEST_PROGS := test_kmod.sh \
test_lirc_mode2.sh \
test_skb_cgroup_id.sh \
test_flow_dissector.sh \
- test_xdp_vlan.sh \
+ test_xdp_vlan_mode_generic.sh \
+ test_xdp_vlan_mode_native.sh \
test_lwt_ip_encap.sh \
test_tcp_check_syncookie.sh \
test_tc_tunnel.sh \
diff --git a/tools/testing/selftests/bpf/test_xdp_vlan.sh b/tools/testing/selftests/bpf/test_xdp_vlan.sh
index c8aed63b0ffe..7348661be815 100755
--- a/tools/testing/selftests/bpf/test_xdp_vlan.sh
+++ b/tools/testing/selftests/bpf/test_xdp_vlan.sh
@@ -2,7 +2,10 @@
# SPDX-License-Identifier: GPL-2.0
# Author: Jesper Dangaard Brouer <hawk@kernel.org>
-TESTNAME=xdp_vlan
+# Allow wrapper scripts to name test
+if [ -z "$TESTNAME" ]; then
+ TESTNAME=xdp_vlan
+fi
# Default XDP mode
XDP_MODE=xdpgeneric
diff --git a/tools/testing/selftests/bpf/test_xdp_vlan_mode_generic.sh b/tools/testing/selftests/bpf/test_xdp_vlan_mode_generic.sh
new file mode 100755
index 000000000000..c515326d6d59
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_xdp_vlan_mode_generic.sh
@@ -0,0 +1,9 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Exit on failure
+set -e
+
+# Wrapper script to test generic-XDP
+export TESTNAME=xdp_vlan_mode_generic
+./test_xdp_vlan.sh --mode=xdpgeneric
diff --git a/tools/testing/selftests/bpf/test_xdp_vlan_mode_native.sh b/tools/testing/selftests/bpf/test_xdp_vlan_mode_native.sh
new file mode 100755
index 000000000000..5cf7ce1f16c1
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_xdp_vlan_mode_native.sh
@@ -0,0 +1,9 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Exit on failure
+set -e
+
+# Wrapper script to test native-XDP
+export TESTNAME=xdp_vlan_mode_native
+./test_xdp_vlan.sh --mode=xdpdrv
^ permalink raw reply related
* [net v1 PATCH 1/4] bpf: fix XDP vlan selftests test_xdp_vlan.sh
From: Jesper Dangaard Brouer @ 2019-08-01 18:00 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: xdp-newbies, Daniel Borkmann, brandon.cazander,
Alexei Starovoitov, Jesper Dangaard Brouer
In-Reply-To: <156468229108.27559.2443904494495785131.stgit@firesoul>
Change BPF selftest test_xdp_vlan.sh to (default) use generic XDP.
This selftest was created together with a fix for generic XDP, in commit
297249569932 ("net: fix generic XDP to handle if eth header was
mangled"). And was suppose to catch if generic XDP was broken again.
The tests are using veth and assumed that veth driver didn't support
native driver XDP, thus it used the (ip link set) 'xdp' attach that fell
back to generic-XDP. But veth gained native-XDP support in 948d4f214fde
("veth: Add driver XDP"), which caused this test script to use
native-XDP.
Fixes: 948d4f214fde ("veth: Add driver XDP")
Fixes: 97396ff0bc2d ("selftests/bpf: add XDP selftests for modifying and popping VLAN headers")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
tools/testing/selftests/bpf/test_xdp_vlan.sh | 42 ++++++++++++++++++++++----
1 file changed, 36 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_xdp_vlan.sh b/tools/testing/selftests/bpf/test_xdp_vlan.sh
index 51a3a31d1aac..c8aed63b0ffe 100755
--- a/tools/testing/selftests/bpf/test_xdp_vlan.sh
+++ b/tools/testing/selftests/bpf/test_xdp_vlan.sh
@@ -1,7 +1,12 @@
#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Author: Jesper Dangaard Brouer <hawk@kernel.org>
TESTNAME=xdp_vlan
+# Default XDP mode
+XDP_MODE=xdpgeneric
+
usage() {
echo "Testing XDP + TC eBPF VLAN manipulations: $TESTNAME"
echo ""
@@ -9,9 +14,23 @@ usage() {
echo " -v | --verbose : Verbose"
echo " --flush : Flush before starting (e.g. after --interactive)"
echo " --interactive : Keep netns setup running after test-run"
+ echo " --mode=XXX : Choose XDP mode (xdp | xdpgeneric | xdpdrv)"
echo ""
}
+valid_xdp_mode()
+{
+ local mode=$1
+
+ case "$mode" in
+ xdpgeneric | xdpdrv | xdp)
+ return 0
+ ;;
+ *)
+ return 1
+ esac
+}
+
cleanup()
{
local status=$?
@@ -37,7 +56,7 @@ cleanup()
# Using external program "getopt" to get --long-options
OPTIONS=$(getopt -o hvfi: \
- --long verbose,flush,help,interactive,debug -- "$@")
+ --long verbose,flush,help,interactive,debug,mode: -- "$@")
if (( $? != 0 )); then
usage
echo "selftests: $TESTNAME [FAILED] Error calling getopt, unknown option?"
@@ -60,6 +79,11 @@ while true; do
cleanup
shift
;;
+ --mode )
+ shift
+ XDP_MODE=$1
+ shift
+ ;;
-- )
shift
break
@@ -81,8 +105,14 @@ if [ "$EUID" -ne 0 ]; then
exit 1
fi
-ip link set dev lo xdp off 2>/dev/null > /dev/null
-if [ $? -ne 0 ];then
+valid_xdp_mode $XDP_MODE
+if [ $? -ne 0 ]; then
+ echo "selftests: $TESTNAME [FAILED] unknown XDP mode ($XDP_MODE)"
+ exit 1
+fi
+
+ip link set dev lo xdpgeneric off 2>/dev/null > /dev/null
+if [ $? -ne 0 ]; then
echo "selftests: $TESTNAME [SKIP] need ip xdp support"
exit 0
fi
@@ -166,7 +196,7 @@ export FILE=test_xdp_vlan.o
# First test: Remove VLAN by setting VLAN ID 0, using "xdp_vlan_change"
export XDP_PROG=xdp_vlan_change
-ip netns exec ns1 ip link set $DEVNS1 xdp object $FILE section $XDP_PROG
+ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE object $FILE section $XDP_PROG
# In ns1: egress use TC to add back VLAN tag 4011
# (del cmd)
@@ -187,8 +217,8 @@ ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
# ETH_P_8021Q indication, and this cause overwriting of our changes.
#
export XDP_PROG=xdp_vlan_remove_outer2
-ip netns exec ns1 ip link set $DEVNS1 xdp off
-ip netns exec ns1 ip link set $DEVNS1 xdp object $FILE section $XDP_PROG
+ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE off
+ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE object $FILE section $XDP_PROG
# Now the namespaces should still be able reach each-other, test with ping:
ip netns exec ns2 ping -W 2 -c 3 $IPADDR1
^ permalink raw reply related
* [net v1 PATCH 0/4] net: fix regressions for generic-XDP
From: Jesper Dangaard Brouer @ 2019-08-01 18:00 UTC (permalink / raw)
To: netdev, David S. Miller
Cc: xdp-newbies, Daniel Borkmann, brandon.cazander,
Alexei Starovoitov, Jesper Dangaard Brouer
In-Reply-To: <20190731211211.GA87084@multapplied.net>
Thanks to Brandon Cazander, who wrote a very detailed bug report that
even used perf probe's on xdp-newbies mailing list, we discovered that
generic-XDP contains some regressions when using bpf_xdp_adjust_head().
First issue were that my selftests script, that use bpf_xdp_adjust_head(),
by mistake didn't use generic-XDP any-longer. That selftest should have
caught the real regression introduced in commit 458bf2f224f0 ("net: core:
support XDP generic on stacked devices.").
To verify this patchset fix the regressions, you can invoked manually via:
cd tools/testing/selftests/bpf/
sudo ./test_xdp_vlan_mode_generic.sh
sudo ./test_xdp_vlan_mode_native.sh
Link: https://www.spinics.net/lists/xdp-newbies/msg01231.html
Fixes: 458bf2f224f0 ("net: core: support XDP generic on stacked devices.")
Reported by: Brandon Cazander <brandon.cazander@multapplied.net>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
Jesper Dangaard Brouer (4):
bpf: fix XDP vlan selftests test_xdp_vlan.sh
selftests/bpf: add wrapper scripts for test_xdp_vlan.sh
selftests/bpf: reduce time to execute test_xdp_vlan.sh
net: fix bpf_xdp_adjust_head regression for generic-XDP
net/core/dev.c | 15 ++++-
tools/testing/selftests/bpf/Makefile | 3 +
tools/testing/selftests/bpf/test_xdp_vlan.sh | 57 ++++++++++++++++----
.../selftests/bpf/test_xdp_vlan_mode_generic.sh | 9 +++
.../selftests/bpf/test_xdp_vlan_mode_native.sh | 9 +++
5 files changed, 75 insertions(+), 18 deletions(-)
create mode 100755 tools/testing/selftests/bpf/test_xdp_vlan_mode_generic.sh
create mode 100755 tools/testing/selftests/bpf/test_xdp_vlan_mode_native.sh
--
^ permalink raw reply
* Re: [PATCH net] ibmveth: use net_err_ratelimited when set_multicast_list
From: Joe Perches @ 2019-08-01 17:54 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Thomas Falcon, David S . Miller
In-Reply-To: <20190801141006.GS18865@dhcp-12-139.nay.redhat.com>
On Thu, 2019-08-01 at 22:10 +0800, Hangbin Liu wrote:
> On Thu, Aug 01, 2019 at 03:28:43AM -0700, Joe Perches wrote:
> > Perhaps add the netdev_<level>_ratelimited variants and use that instead
> >
> > Somthing like:
>
> Yes, that looks better. Do you mind if I take your code and add your
> Signed-off-by info?
Well, if you test and verify all the paths,
(I did not and just wrote that without testing),
you could add something like "Suggested-by:".
cheers, Joe
> Thanks
> Hangbin
> > ---
> > include/linux/netdevice.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 54 insertions(+)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 88292953aa6f..37116019e14f 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -4737,6 +4737,60 @@ do { \
> > #define netdev_info_once(dev, fmt, ...) \
> > netdev_level_once(KERN_INFO, dev, fmt, ##__VA_ARGS__)
> >
> > +#define netdev_level_ratelimited(netdev_level, dev, fmt, ...) \
> > +do { \
> > + static DEFINE_RATELIMIT_STATE(_rs, \
> > + DEFAULT_RATELIMIT_INTERVAL, \
> > + DEFAULT_RATELIMIT_BURST); \
> > + if (__ratelimit(&_rs)) \
> > + netdev_level(dev, fmt, ##__VA_ARGS__); \
> > +} while (0)
> > +
> > +#define netdev_emerg_ratelimited(dev, fmt, ...) \
> > + netdev_level_ratelimited(netdev_emerg, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_alert_ratelimited(dev, fmt, ...) \
> > + netdev_level_ratelimited(netdev_alert, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_crit_ratelimited(dev, fmt, ...) \
> > + netdev_level_ratelimited(netdev_crit, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_err_ratelimited(dev, fmt, ...) \
> > + netdev_level_ratelimited(netdev_err, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_warn_ratelimited(dev, fmt, ...) \
> > + netdev_level_ratelimited(netdev_warn, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_notice_ratelimited(dev, fmt, ...) \
> > + netdev_level_ratelimited(netdev_notice, dev, fmt, ##__VA_ARGS__)
> > +#define netdev_info_ratelimited(dev, fmt, ...) \
> > + netdev_level_ratelimited(netdev_info, dev, fmt, ##__VA_ARGS__)
> > +
> > +#if defined(CONFIG_DYNAMIC_DEBUG)
> > +/* descriptor check is first to prevent flooding with "callbacks suppressed" */
> > +#define netdev_dbg_ratelimited(dev, fmt, ...) \
> > +do { \
> > + static DEFINE_RATELIMIT_STATE(_rs, \
> > + DEFAULT_RATELIMIT_INTERVAL, \
> > + DEFAULT_RATELIMIT_BURST); \
> > + DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
> > + if (DYNAMIC_DEBUG_BRANCH(descriptor) && \
> > + __ratelimit(&_rs)) \
> > + __dynamic_netdev_dbg(&descriptor, dev, fmt, \
> > + ##__VA_ARGS__); \
> > +} while (0)
> > +#elif defined(DEBUG)
> > +#define netdev_dbg_ratelimited(dev, fmt, ...) \
> > +do { \
> > + static DEFINE_RATELIMIT_STATE(_rs, \
> > + DEFAULT_RATELIMIT_INTERVAL, \
> > + DEFAULT_RATELIMIT_BURST); \
> > + if (__ratelimit(&_rs)) \
> > + netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> > +} while (0)
> > +#else
> > +#define netdev_dbg_ratelimited(dev, fmt, ...) \
> > +do { \
> > + if (0) \
> > + netdev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__); \
> > +} while (0)
> > +#endif
> > +
> > #define MODULE_ALIAS_NETDEV(device) \
> > MODULE_ALIAS("netdev-" device)
> >
> >
> >
^ permalink raw reply
* Re: [PATCH bpf-next v10 10/10] landlock: Add user and kernel documentation for Landlock
From: Randy Dunlap @ 2019-08-01 17:49 UTC (permalink / raw)
To: Mickaël Salaün, Mickaël Salaün, linux-kernel
Cc: Alexander Viro, Alexei Starovoitov, Andrew Morton,
Andy Lutomirski, Arnaldo Carvalho de Melo, Casey Schaufler,
Daniel Borkmann, David Drysdale, David S . Miller,
Eric W . Biederman, James Morris, Jann Horn, John Johansen,
Jonathan Corbet, Kees Cook, Michael Kerrisk, Paul Moore,
Sargun Dhillon, Serge E . Hallyn, Shuah Khan, Stephen Smalley,
Tejun Heo, Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
kernel-hardening, linux-api, linux-fsdevel, linux-security-module,
netdev
In-Reply-To: <2ced8fc8-79a6-b0fb-70fe-6716fae92aa7@ssi.gouv.fr>
On 8/1/19 10:03 AM, Mickaël Salaün wrote:
>>> +Ptrace restrictions
>>> +-------------------
>>> +
>>> +A landlocked process has less privileges than a non-landlocked process and must
>>> +then be subject to additional restrictions when manipulating another process.
>>> +To be allowed to use :manpage:`ptrace(2)` and related syscalls on a target
>>> +process, a landlocked process must have a subset of the target process programs.
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> Maybe that last statement is correct, but it seems to me that it is missing something.
> What about this:
>
> To be allowed to trace a process (using :manpage:`ptrace(2)`), a
> landlocked tracer process must only be constrained by a subset (possibly
> empty) of the Landlock programs which are also applied to the tracee.
> This ensure that the tracer has less or the same constraints than the
ensures
> tracee, hence protecting against privilege escalation.
Yes, better. Thanks.
--
~Randy
^ permalink raw reply
* Re: [PATCH 0/8] net: Manufacturer names and spelling fixes
From: David Miller @ 2019-08-01 17:46 UTC (permalink / raw)
To: geert+renesas; +Cc: netdev, linux-kernel
In-Reply-To: <20190731132216.17194-1-geert+renesas@glider.be>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 31 Jul 2019 15:22:08 +0200
> This is a set of fixes for (some blatantly) wrong manufacturer names
> and various spelling issues, mostly in Kconfig help texts.
Series applied, thank you Geert.
^ permalink raw reply
* Re: [PATCH] net: sctp: Rename fallthrough label to unhandled
From: Joe Perches @ 2019-08-01 17:42 UTC (permalink / raw)
To: Neil Horman
Cc: Vlad Yasevich, Marcelo Ricardo Leitner, David S. Miller,
linux-sctp, netdev, linux-kernel
In-Reply-To: <20190801105051.GA11487@hmswarspite.think-freely.org>
On Thu, 2019-08-01 at 06:50 -0400, Neil Horman wrote:
> On Wed, Jul 31, 2019 at 03:23:46PM -0700, Joe Perches wrote:
[]
> You can say that if you want, but you made the point that your think the macro
> as you have written is more readable. You example illustrates though that /*
> fallthrough */ is a pretty common comment, and not prefixing it makes it look
> like someone didn't add a comment that they meant to. The __ prefix is standard
> practice for defining macros to attributes (212 instances of it by my count). I
> don't mind rewriting the goto labels at all, but I think consistency is
> valuable.
Hey Neil.
Perhaps you want to make this argument on the RFC patch thread
that introduces the fallthrough pseudo-keyword.
https://lore.kernel.org/patchwork/patch/1108577/
^ permalink raw reply
* Re: [net-next 00/16][pull request] 100GbE Intel Wired LAN Driver Updates 2019-07-31
From: David Miller @ 2019-08-01 17:42 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann
In-Reply-To: <20190731204147.8582-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 31 Jul 2019 13:41:31 -0700
> This series contains updates to ice driver only.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH bpf-next v10 06/10] bpf,landlock: Add a new map type: inode
From: Alexei Starovoitov @ 2019-08-01 17:35 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Mickaël Salaün, LKML, Alexander Viro,
Alexei Starovoitov, Andrew Morton, Andy Lutomirski,
Arnaldo Carvalho de Melo, Casey Schaufler, Daniel Borkmann,
David Drysdale, David S . Miller, Eric W . Biederman,
James Morris, Jann Horn, John Johansen, Jonathan Corbet,
Kees Cook, Michael Kerrisk, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
Kernel Hardening, Linux API, Linux-Fsdevel, LSM List,
Network Development
In-Reply-To: <59e8fab9-34df-0ebe-ca6b-8b34bf582b75@ssi.gouv.fr>
On Wed, Jul 31, 2019 at 09:11:10PM +0200, Mickaël Salaün wrote:
>
>
> On 31/07/2019 20:58, Alexei Starovoitov wrote:
> > On Wed, Jul 31, 2019 at 11:46 AM Mickaël Salaün
> > <mickael.salaun@ssi.gouv.fr> wrote:
> >>>> + for (i = 0; i < htab->n_buckets; i++) {
> >>>> + head = select_bucket(htab, i);
> >>>> + hlist_nulls_for_each_entry_safe(l, n, head, hash_node) {
> >>>> + landlock_inode_remove_map(*((struct inode **)l->key), map);
> >>>> + }
> >>>> + }
> >>>> + htab_map_free(map);
> >>>> +}
> >>>
> >>> user space can delete the map.
> >>> that will trigger inode_htab_map_free() which will call
> >>> landlock_inode_remove_map().
> >>> which will simply itereate the list and delete from the list.
> >>
> >> landlock_inode_remove_map() removes the reference to the map (being
> >> freed) from the inode (with an RCU lock).
> >
> > I'm going to ignore everything else for now and focus only on this bit,
> > since it's fundamental issue to address before this discussion can
> > go any further.
> > rcu_lock is not a spin_lock. I'm pretty sure you know this.
> > But you're arguing that it's somehow protecting from the race
> > I mentioned above?
> >
>
> I was just clarifying your comment to avoid misunderstanding about what
> is being removed.
>
> As said in the full response, there is currently a race but, if I add a
> bpf_map_inc() call when the map is referenced by inode->security, then I
> don't see how a race could occur because such added map could only be
> freed in a security_inode_free() (as long as it retains a reference to
> this inode).
then it will be a cycle and a map will never be deleted?
closing map_fd should delete a map. It cannot be alive if it's not
pinned in bpffs, there are no FDs that are holding it, and no progs using it.
So the map deletion will iterate over inodes that belong to this map.
In parallel security_inode_free() will be called that will iterate
over its link list that contains elements from different maps.
So the same link list is modified by two cpus.
Where is a lock that protects from concurrent links list manipulations?
> Les données à caractère personnel recueillies et traitées dans le cadre de cet échange, le sont à seule fin d’exécution d’une relation professionnelle et s’opèrent dans cette seule finalité et pour la durée nécessaire à cette relation. Si vous souhaitez faire usage de vos droits de consultation, de rectification et de suppression de vos données, veuillez contacter contact.rgpd@sgdsn.gouv.fr. Si vous avez reçu ce message par erreur, nous vous remercions d’en informer l’expéditeur et de détruire le message. The personal data collected and processed during this exchange aims solely at completing a business relationship and is limited to the necessary duration of that relationship. If you wish to use your rights of consultation, rectification and deletion of your data, please contact: contact.rgpd@sgdsn.gouv.fr. If you have received this message in error, we thank you for informing the sender and destroying the message.
Please get rid of this. It's absolutely not appropriate on public mailing list.
Next time I'd have to ignore emails that contain such disclaimers.
^ permalink raw reply
* Re: [PATCH net-next 00/12] net: hns3: some code optimizations & bugfixes & features
From: David Miller @ 2019-08-01 17:32 UTC (permalink / raw)
To: tanhuazhong; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <1564631745-36733-1-git-send-email-tanhuazhong@huawei.com>
From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Thu, 1 Aug 2019 11:55:33 +0800
> This patch-set includes code optimizations, bugfixes and features for
> the HNS3 ethernet controller driver.
These look good, series applied, thanks.
^ permalink raw reply
* Re: [PATCH v1 2/2] net: npcm: add NPCM7xx EMC 10/100 Ethernet driver
From: Willem de Bruijn @ 2019-08-01 17:25 UTC (permalink / raw)
To: Avi Fishman
Cc: Patrick Venture, Nancy Yuen, benjaminfair, David Miller, robh+dt,
mark.rutland, Greg Kroah-Hartman, tmaimon77, tali.perry1, openbmc,
Network Development, devicetree, linux-kernel, tglx
In-Reply-To: <20190801072611.27935-3-avifishman70@gmail.com>
On Thu, Aug 1, 2019 at 3:28 AM Avi Fishman <avifishman70@gmail.com> wrote:
>
> EMC Ethernet Media Access Controller supports 10/100 Mbps and
> RMII.
> This driver has been working on Nuvoton BMC NPCM7xx.
>
> Signed-off-by: Avi Fishman <avifishman70@gmail.com>
> +/* global setting for driver */
> +#define RX_QUEUE_LEN 128
> +#define TX_QUEUE_LEN 64
> +#define MAX_RBUFF_SZ 0x600
> +#define MAX_TBUFF_SZ 0x600
> +#define TX_TIMEOUT 50
> +#define DELAY 1000
> +#define CAM0 0x0
> +#define RX_POLL_SIZE 16
> +
> +#ifdef CONFIG_VLAN_8021Q
> +#define IS_VLAN 1
> +#else
> +#define IS_VLAN 0
> +#endif
> +
> +#define MAX_PACKET_SIZE (1514 + (IS_VLAN * 4))
1514 -> ETH_FRAME_LEN
4 -> VLAN_HLEN
Does this device support stacked VLAN?
Is this really the device maximum?
> +#define MAX_PACKET_SIZE_W_CRC (MAX_PACKET_SIZE + 4) /* 1518 */
4 -> ETH_FCS_LEN
> +#if defined CONFIG_NPCM7XX_EMC_ETH_DEBUG || defined CONFIG_DEBUG_FS
> +#define REG_PRINT(reg_name) {t = scnprintf(next, size, "%-10s = %08X\n", \
> + #reg_name, readl(ether->reg + (reg_name))); size -= t; next += t; }
> +#define DUMP_PRINT(f, x...) {t = scnprintf(next, size, f, ## x); size -= t; \
> + next += t; }
> +
> +static int npcm7xx_info_dump(char *buf, int count, struct net_device *netdev)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct npcm7xx_txbd *txbd;
> + struct npcm7xx_rxbd *rxbd;
> + unsigned long flags;
> + unsigned int i, cur, txd_offset, rxd_offset;
> + char *next = buf;
> + unsigned int size = count;
> + int t;
> + int is_locked = spin_is_locked(ðer->lock);
> +
> + if (!is_locked)
> + spin_lock_irqsave(ðer->lock, flags);
> +
> + /* ------basic driver information ---- */
> + DUMP_PRINT("NPCM7XX EMC %s driver version: %s\n", netdev->name,
> + DRV_MODULE_VERSION);
> +
> + REG_PRINT(REG_CAMCMR);
> + REG_PRINT(REG_CAMEN);
> + REG_PRINT(REG_CAMM_BASE);
> + REG_PRINT(REG_CAML_BASE);
> + REG_PRINT(REG_TXDLSA);
> + REG_PRINT(REG_RXDLSA);
> + REG_PRINT(REG_MCMDR);
> + REG_PRINT(REG_MIID);
> + REG_PRINT(REG_MIIDA);
> + REG_PRINT(REG_FFTCR);
> + REG_PRINT(REG_TSDR);
> + REG_PRINT(REG_RSDR);
> + REG_PRINT(REG_DMARFC);
> + REG_PRINT(REG_MIEN);
> + REG_PRINT(REG_MISTA);
> + REG_PRINT(REG_MGSTA);
> + REG_PRINT(REG_MPCNT);
> + writel(0x7FFF, (ether->reg + REG_MPCNT));
> + REG_PRINT(REG_MRPC);
> + REG_PRINT(REG_MRPCC);
> + REG_PRINT(REG_MREPC);
> + REG_PRINT(REG_DMARFS);
> + REG_PRINT(REG_CTXDSA);
> + REG_PRINT(REG_CTXBSA);
> + REG_PRINT(REG_CRXDSA);
> + REG_PRINT(REG_CRXBSA);
> + REG_PRINT(REG_RXFSM);
> + REG_PRINT(REG_TXFSM);
> + REG_PRINT(REG_FSM0);
> + REG_PRINT(REG_FSM1);
> + REG_PRINT(REG_DCR);
> + REG_PRINT(REG_DMMIR);
> + REG_PRINT(REG_BISTR);
> + DUMP_PRINT("\n");
> +
> + DUMP_PRINT("netif_queue %s\n\n", netif_queue_stopped(netdev) ?
> + "Stopped" : "Running");
> + if (ether->rdesc)
> + DUMP_PRINT("napi is %s\n\n", test_bit(NAPI_STATE_SCHED,
> + ðer->napi.state) ?
> + "scheduled" :
> + "not scheduled");
> +
> + txd_offset = (readl((ether->reg + REG_CTXDSA)) -
> + readl((ether->reg + REG_TXDLSA))) /
> + sizeof(struct npcm7xx_txbd);
> + DUMP_PRINT("TXD offset %6d\n", txd_offset);
> + DUMP_PRINT("cur_tx %6d\n", ether->cur_tx);
> + DUMP_PRINT("finish_tx %6d\n", ether->finish_tx);
> + DUMP_PRINT("pending_tx %6d\n", ether->pending_tx);
> + /* debug counters */
> + DUMP_PRINT("tx_tdu %6d\n", ether->tx_tdu);
> + ether->tx_tdu = 0;
> + DUMP_PRINT("tx_tdu_i %6d\n", ether->tx_tdu_i);
> + ether->tx_tdu_i = 0;
> + DUMP_PRINT("tx_cp_i %6d\n", ether->tx_cp_i);
> + ether->tx_cp_i = 0;
> + DUMP_PRINT("tx_int_count %6d\n", ether->tx_int_count);
> + ether->tx_int_count = 0;
> + DUMP_PRINT("count_xmit tx %6d\n", ether->count_xmit);
> + ether->count_xmit = 0;
> + DUMP_PRINT("count_finish %6d\n", ether->count_finish);
> + ether->count_finish = 0;
> + DUMP_PRINT("\n");
> +
> + rxd_offset = (readl((ether->reg + REG_CRXDSA)) -
> + readl((ether->reg + REG_RXDLSA)))
> + / sizeof(struct npcm7xx_txbd);
> + DUMP_PRINT("RXD offset %6d\n", rxd_offset);
> + DUMP_PRINT("cur_rx %6d\n", ether->cur_rx);
> + DUMP_PRINT("rx_err %6d\n", ether->rx_err);
> + ether->rx_err = 0;
> + DUMP_PRINT("rx_berr %6d\n", ether->rx_berr);
> + ether->rx_berr = 0;
> + DUMP_PRINT("rx_stuck %6d\n", ether->rx_stuck);
> + ether->rx_stuck = 0;
> + DUMP_PRINT("rdu %6d\n", ether->rdu);
> + ether->rdu = 0;
> + DUMP_PRINT("rxov rx %6d\n", ether->rxov);
> + ether->rxov = 0;
> + /* debug counters */
> + DUMP_PRINT("rx_int_count %6d\n", ether->rx_int_count);
> + ether->rx_int_count = 0;
> + DUMP_PRINT("rx_err_count %6d\n", ether->rx_err_count);
> + ether->rx_err_count = 0;
Basic counters like tx_packets and rx_errors are probably better
exported regardless of debug level as net_device_stats. And then don't
need to be copied in debug output.
Less standard counters like tx interrupt count are probably better
candidates for ethtool -S.
> +#ifdef CONFIG_NPCM7XX_EMC_ETH_DEBUG
> +static void npcm7xx_info_print(struct net_device *netdev)
> +{
> + char *emc_dump_buf;
> + int count;
> + struct npcm7xx_ether *ether;
> + struct platform_device *pdev;
> + char c;
> + char *tmp_buf;
> + const size_t print_size = 5 * PAGE_SIZE;
> +
> + ether = netdev_priv(netdev);
> + pdev = ether->pdev;
> +
> + emc_dump_buf = kmalloc(print_size, GFP_KERNEL);
> + if (!emc_dump_buf)
> + return;
> +
> + tmp_buf = emc_dump_buf;
> + count = npcm7xx_info_dump(emc_dump_buf, print_size, netdev);
> + while (count > 512) {
> + c = tmp_buf[512];
> + tmp_buf[512] = 0;
> + dev_info(&pdev->dev, "%s", tmp_buf);
> + tmp_buf += 512;
> + tmp_buf[0] = c;
> + count -= 512;
Missing closing parenthesis.
Also, why this buffering to printk?
> +static void npcm7xx_write_cam(struct net_device *netdev,
> + unsigned int x, unsigned char *pval)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + __le32 msw, lsw;
> +
> + msw = (pval[0] << 24) | (pval[1] << 16) | (pval[2] << 8) | pval[3];
> +
> + lsw = (pval[4] << 24) | (pval[5] << 16);
Does __le32 plus this explicit shifting define host endianness? Better
to keep independent?
> +
> + writel(lsw, (ether->reg + REG_CAML_BASE) + x * CAM_ENTRY_SIZE);
> + writel(msw, (ether->reg + REG_CAMM_BASE) + x * CAM_ENTRY_SIZE);
> + dev_dbg(ðer->pdev->dev,
> + "REG_CAML_BASE = 0x%08X REG_CAMM_BASE = 0x%08X", lsw, msw);
> +}
> +
> +static struct sk_buff *get_new_skb(struct net_device *netdev, u32 i)
> +{
> + __le32 buffer;
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct sk_buff *skb = netdev_alloc_skb(netdev,
> + roundup(MAX_PACKET_SIZE_W_CRC, 4));
> +
> + if (unlikely(!skb)) {
> + if (net_ratelimit())
> + netdev_warn(netdev, "failed to allocate rx skb\n");
can use net_warn_ratelimited (here and elsewhere)
> + buffer = ether->rx_scratch_dma;
> + } else {
> + /* Do not unmark the following skb_reserve() Receive Buffer
> + * Starting Address must be aligned to 4 bytes and the following
> + * line if unmarked will make it align to 2 and this likely will
> + * hult the RX and crash the linux
halt?
> + * skb_reserve(skb, NET_IP_ALIGN);
> + */
> + skb->dev = netdev;
> + buffer = dma_map_single(&netdev->dev,
> + skb->data,
> + roundup(MAX_PACKET_SIZE_W_CRC, 4),
> + DMA_FROM_DEVICE);
> + if (unlikely(dma_mapping_error(&netdev->dev, buffer))) {
> + if (net_ratelimit())
> + netdev_err(netdev, "failed to map rx page\n");
> + dev_kfree_skb_any(skb);
> + buffer = ether->rx_scratch_dma;
> + skb = NULL;
> + }
> + }
> + ether->rx_skb[i] = skb;
> + ether->rdesc[i].buffer = buffer;
> +
> + return skb;
> +}
> +
> +static int npcm7xx_ether_close(struct net_device *netdev)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> +
> + npcm7xx_return_default_idle(netdev);
> +
> + if (ether->phy_dev)
> + phy_stop(ether->phy_dev);
> + else if (ether->use_ncsi)
> + ncsi_stop_dev(ether->ncsidev);
> +
> + msleep(20);
> +
> + free_irq(ether->txirq, netdev);
> + free_irq(ether->rxirq, netdev);
> +
> + netif_stop_queue(netdev);
> + napi_disable(ðer->napi);
Cleanup state in reverse of allocation.
> +static int npcm7xx_ether_start_xmit(struct sk_buff *skb, struct net_device *netdev)
> +{
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct npcm7xx_txbd *txbd;
> + unsigned long flags;
> +
> + ether->count_xmit++;
> +
> + /* Insert new buffer */
> + txbd = (ether->tdesc + ether->cur_tx);
> + txbd->buffer = dma_map_single(&netdev->dev, skb->data, skb->len,
> + DMA_TO_DEVICE);
> + ether->tx_skb[ether->cur_tx] = skb;
> + if (skb->len > MAX_PACKET_SIZE)
> + dev_err(ðer->pdev->dev,
> + "skb->len (= %d) > MAX_PACKET_SIZE (= %d)\n",
> + skb->len, MAX_PACKET_SIZE);
> + txbd->sl = skb->len > MAX_PACKET_SIZE ? MAX_PACKET_SIZE : skb->len;
Check for errors before mapping to device, and drop packet? Probably
don't want to output truncated packets.
Also rate limit such messages.
> + dma_wmb();
> +
> + txbd->mode = TX_OWN_DMA | PADDINGMODE | CRCMODE;
> +
> + /* make sure that data is in memory before we trigger TX */
> + wmb();
> +
> + /* trigger TX */
> + writel(ENSTART, (ether->reg + REG_TSDR));
> +
> + if (++ether->cur_tx >= TX_QUEUE_LEN)
> + ether->cur_tx = 0;
> +
> + spin_lock_irqsave(ðer->lock, flags);
> + ether->pending_tx++;
> +
> + /* sometimes we miss the tx interrupt due to HW issue, so NAPI will not
> + * clean the pending tx, so we clean it also here
> + */
> + npcm7xx_clean_tx(netdev, true);
> +
> + if (ether->pending_tx >= TX_QUEUE_LEN - 1) {
> + __le32 reg_mien;
> + unsigned int index_to_wake = ether->cur_tx +
> + ((TX_QUEUE_LEN * 3) / 4);
> +
> + if (index_to_wake >= TX_QUEUE_LEN)
> + index_to_wake -= TX_QUEUE_LEN;
> +
> + txbd = (ether->tdesc + index_to_wake);
> + txbd->mode = TX_OWN_DMA | PADDINGMODE | CRCMODE | MACTXINTEN;
> +
> + /* make sure that data is in memory before we trigger TX */
> + wmb();
> +
> + /* Clear TDU interrupt */
> + writel(MISTA_TDU, (ether->reg + REG_MISTA));
> +
> + /* due to HW issue somtimes, we miss the TX interrupt we just
somtimes -> sometimes
> + * set (MACTXINTEN), so we also set TDU for Transmit
> + * Descriptor Unavailable interrupt
> + */
> + reg_mien = readl((ether->reg + REG_MIEN));
> + if (reg_mien != 0)
> + /* Enable TDU interrupt */
> + writel(reg_mien | ENTDU, (ether->reg + REG_MIEN));
> +
> + ether->tx_tdu++;
> + netif_stop_queue(netdev);
> + }
> +
> + spin_unlock_irqrestore(ðer->lock, flags);
> +
> + return 0;
> +}
> +
> +static irqreturn_t npcm7xx_tx_interrupt(int irq, void *dev_id)
> +{
> + struct npcm7xx_ether *ether;
> + struct platform_device *pdev;
> + struct net_device *netdev;
> + __le32 status;
> + unsigned long flags;
> +
> + netdev = dev_id;
> + ether = netdev_priv(netdev);
> + pdev = ether->pdev;
> +
> + npcm7xx_get_and_clear_int(netdev, &status, 0xFFFF0000);
> +
> + ether->tx_int_count++;
> +
> + if (status & MISTA_EXDEF)
> + dev_err(&pdev->dev, "emc defer exceed interrupt status=0x%08X\n"
> + , status);
> + else if (status & MISTA_TXBERR) {
> + dev_err(&pdev->dev, "emc bus error interrupt status=0x%08X\n",
> + status);
> +#ifdef CONFIG_NPCM7XX_EMC_ETH_DEBUG
> + npcm7xx_info_print(netdev);
> +#endif
> + spin_lock_irqsave(ðer->lock, flags);
irqsave in hard interrupt context?
> + writel(0, (ether->reg + REG_MIEN)); /* disable any interrupt */
> + spin_unlock_irqrestore(ðer->lock, flags);
> + ether->need_reset = 1;
> + } else if (status & ~(MISTA_TXINTR | MISTA_TXCP | MISTA_TDU))
> + dev_err(&pdev->dev, "emc other error interrupt status=0x%08X\n",
> + status);
> +
> + /* if we got MISTA_TXCP | MISTA_TDU remove those interrupt and call napi */
The goal of napi is to keep interrupts disabled until napi completes.
> + if (status & (MISTA_TXCP | MISTA_TDU) &
> + readl((ether->reg + REG_MIEN))) {
> + __le32 reg_mien;
> +
> + spin_lock_irqsave(ðer->lock, flags);
> + reg_mien = readl((ether->reg + REG_MIEN));
> + if (reg_mien & ENTDU)
> + /* Disable TDU interrupt */
> + writel(reg_mien & (~ENTDU), (ether->reg + REG_MIEN));
> +
> + spin_unlock_irqrestore(ðer->lock, flags);
> +
> + if (status & MISTA_TXCP)
> + ether->tx_cp_i++;
> + if (status & MISTA_TDU)
> + ether->tx_tdu_i++;
> + } else {
> + dev_dbg(&pdev->dev, "status=0x%08X\n", status);
> + }
> +
> + napi_schedule(ðer->napi);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t npcm7xx_rx_interrupt(int irq, void *dev_id)
> +{
> + struct net_device *netdev = (struct net_device *)dev_id;
> + struct npcm7xx_ether *ether = netdev_priv(netdev);
> + struct platform_device *pdev = ether->pdev;
> + __le32 status;
> + unsigned long flags;
> + unsigned int any_err = 0;
> + __le32 rxfsm;
> +
> + npcm7xx_get_and_clear_int(netdev, &status, 0xFFFF);
Same here
> +static int npcm7xx_poll(struct napi_struct *napi, int budget)
> +{
> + struct npcm7xx_ether *ether =
> + container_of(napi, struct npcm7xx_ether, napi);
> + struct npcm7xx_rxbd *rxbd;
> + struct net_device *netdev = ether->netdev;
> + struct platform_device *pdev = ether->pdev;
> + struct sk_buff *s;
> + unsigned int length;
> + __le32 status;
> + unsigned long flags;
> + int rx_cnt = 0;
> + int complete = 0;
> + unsigned int rx_offset = (readl((ether->reg + REG_CRXDSA)) -
> + ether->start_rx_ptr) /
> + sizeof(struct npcm7xx_txbd);
> + unsigned int local_count = (rx_offset >= ether->cur_rx) ?
> + rx_offset - ether->cur_rx : rx_offset +
> + RX_QUEUE_LEN - ether->cur_rx;
> +
> + if (local_count > ether->max_waiting_rx)
> + ether->max_waiting_rx = local_count;
> +
> + if (local_count > (4 * RX_POLL_SIZE))
> + /* we are porbably in a storm of short packets and we don't
porbably - probably
> + * want to get into RDU since short packets in RDU cause
> + * many RXOV which may cause EMC halt, so we filter out all
> + * coming packets
> + */
> + writel(0, (ether->reg + REG_CAMCMR));
> +
> + if (local_count <= budget)
> + /* we can restore accepting of packets */
> + writel(ether->camcmr, (ether->reg + REG_CAMCMR));
> +
> + spin_lock_irqsave(ðer->lock, flags);
> + npcm7xx_clean_tx(netdev, false);
> + spin_unlock_irqrestore(ðer->lock, flags);
> +
> + rxbd = (ether->rdesc + ether->cur_rx);
> +
> + while (rx_cnt < budget) {
> + status = rxbd->sl;
> + if ((status & RX_OWN_DMA) == RX_OWN_DMA) {
> + complete = 1;
> + break;
> + }
> + /* for debug puposes we save the previous value */
puposes -> purposes
> + rxbd->reserved = status;
> + s = ether->rx_skb[ether->cur_rx];
> + length = status & 0xFFFF;
> +
> + /* If VLAN is not supporte RXDS_PTLE (packet too long) is also
supporte -> supported (stopping pointing out typos after this).
> +static const struct net_device_ops npcm7xx_ether_netdev_ops = {
> + .ndo_open = npcm7xx_ether_open,
> + .ndo_stop = npcm7xx_ether_close,
> + .ndo_start_xmit = npcm7xx_ether_start_xmit,
> + .ndo_get_stats = npcm7xx_ether_stats,
> + .ndo_set_rx_mode = npcm7xx_ether_set_rx_mode,
> + .ndo_set_mac_address = npcm7xx_set_mac_address,
> + .ndo_do_ioctl = npcm7xx_ether_ioctl,
> + .ndo_validate_addr = eth_validate_addr,
> + .ndo_change_mtu = eth_change_mtu,
This is marked as deprecated. Also in light of the hardcoded
MAX_PACKET_SIZE, probably want to set dev->max_mtu.
> +static int npcm7xx_ether_probe(struct platform_device *pdev)
> +{
> + struct npcm7xx_ether *ether;
> + struct net_device *netdev;
> + int error;
> +
> + struct clk *emc_clk = NULL;
> + struct device_node *np = pdev->dev.of_node;
> +
> + pdev->id = of_alias_get_id(np, "ethernet");
> + if (pdev->id < 0)
> + pdev->id = 0;
> +
> + emc_clk = devm_clk_get(&pdev->dev, NULL);
> +
> + if (IS_ERR(emc_clk))
> + return PTR_ERR(emc_clk);
> +
> + /* Enable Clock */
> + clk_prepare_enable(emc_clk);
> +
> + error = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> + if (error)
> + return -ENODEV;
> +
> + netdev = alloc_etherdev(sizeof(struct npcm7xx_ether));
> + if (!netdev)
> + return -ENOMEM;
> +
> + ether = netdev_priv(netdev);
> +
> + ether->reset = devm_reset_control_get(&pdev->dev, NULL);
> + if (IS_ERR(ether->reset))
> + return PTR_ERR(ether->reset);
Memory leak on error path
Also missing netif_napi_del in npcm7xx_ether_remove?
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: mt7530: Convert to PHYLINK API
From: David Miller @ 2019-08-01 17:22 UTC (permalink / raw)
To: opensource
Cc: netdev, frank-w, sean.wang, f.fainelli, matthias.bgg, andrew,
vivien.didelot, john, linux-mediatek, linux-mips, robh+dt,
devicetree, linux
In-Reply-To: <20190801172104.Horde.Cuwt4jywUX_mcO9-n8QpWTN@www.vdorst.com>
From: René van Dorst <opensource@vdorst.com>
Date: Thu, 01 Aug 2019 17:21:04 +0000
> Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
>
>> Hi,
>>
>> Just a couple of minor points.
>>
>> On Wed, Jul 24, 2019 at 09:25:47PM +0200, René van Dorst wrote:
>
> <snip>
>
>>> +
>>> +static void mt7530_phylink_validate(struct dsa_switch *ds, int port,
>>> + unsigned long *supported,
>>> + struct phylink_link_state *state)
>>> +{
>>> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>>> +
>>> + switch (port) {
>>> + case 0: /* Internal phy */
>>> + case 1:
>>> + case 2:
>>> + case 3:
>>> + case 4:
>>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>>> + state->interface != PHY_INTERFACE_MODE_GMII)
>>> + goto unsupported;
>>> + break;
>>> + /* case 5: Port 5 not supported! */
>>> + case 6: /* 1st cpu port */
>>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>>> + state->interface != PHY_INTERFACE_MODE_RGMII &&
>>> + state->interface != PHY_INTERFACE_MODE_TRGMII)
>>> + goto unsupported;
>>> + break;
>>> + default:
>>> + linkmode_zero(supported);
>>> + dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>>
>> You could reorder this as:
>>
>> default:
>> dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>> unsupported:
>> linkmode_zero(supported);
>>
>
> Hi David,
>
>> and save having the "unsupported" at the end of the function. Not
>> sure
>> what DaveM would think of it though.
>
> Can you give your option about this?
> So I know what to do with it and make a new series.
Russell's suggestion is fine.
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: dsa: mt7530: Convert to PHYLINK API
From: René van Dorst @ 2019-08-01 17:21 UTC (permalink / raw)
To: davem
Cc: netdev, frank-w, sean.wang, f.fainelli, matthias.bgg, andrew,
vivien.didelot, john, linux-mediatek, linux-mips, robh+dt,
devicetree, Russell King - ARM Linux admin
In-Reply-To: <20190727184828.GT1330@shell.armlinux.org.uk>
Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
> Hi,
>
> Just a couple of minor points.
>
> On Wed, Jul 24, 2019 at 09:25:47PM +0200, René van Dorst wrote:
<snip>
>> +
>> +static void mt7530_phylink_validate(struct dsa_switch *ds, int port,
>> + unsigned long *supported,
>> + struct phylink_link_state *state)
>> +{
>> + __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>> +
>> + switch (port) {
>> + case 0: /* Internal phy */
>> + case 1:
>> + case 2:
>> + case 3:
>> + case 4:
>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>> + state->interface != PHY_INTERFACE_MODE_GMII)
>> + goto unsupported;
>> + break;
>> + /* case 5: Port 5 not supported! */
>> + case 6: /* 1st cpu port */
>> + if (state->interface != PHY_INTERFACE_MODE_NA &&
>> + state->interface != PHY_INTERFACE_MODE_RGMII &&
>> + state->interface != PHY_INTERFACE_MODE_TRGMII)
>> + goto unsupported;
>> + break;
>> + default:
>> + linkmode_zero(supported);
>> + dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
>
> You could reorder this as:
>
> default:
> dev_err(ds->dev, "%s: unsupported port: %i\n", __func__, port);
> unsupported:
> linkmode_zero(supported);
>
Hi David,
> and save having the "unsupported" at the end of the function. Not sure
> what DaveM would think of it though.
Can you give your option about this?
So I know what to do with it and make a new series.
Greats,
René
>
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
^ 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