Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] perf: arm-spe: Fix addresses of synthesized SPE events
From: Leo Yan @ 2022-04-24 15:22 UTC (permalink / raw)
  To: Timothy Hayes
  Cc: linux-kernel, linux-perf-users, acme, John Garry, Will Deacon,
	Mathieu Poirier, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, linux-arm-kernel, netdev, bpf
In-Reply-To: <20220424122831.GC978927@leoy-ThinkPad-X240s>

On Sun, Apr 24, 2022 at 08:28:31PM +0800, Leo Yan wrote:
> On Thu, Apr 21, 2022 at 05:52:03PM +0100, Timothy Hayes wrote:
> > This patch corrects a bug whereby synthesized events from SPE
> > samples are missing virtual addresses.
> > 
> > Signed-off-by: Timothy Hayes <timothy.hayes@arm.com>
> 
> Reviewed-by: Leo Yan <leo.yan@linaro.org>

Supplement for fixed tag:

Since patches 01, 02 are fixing bugs, please add fixed tag [1]:

Fixes: 54f7815efef7 ("perf arm-spe: Fill address info for samples")

Thanks,
Leo

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst#n138

^ permalink raw reply

* [PATCH bpf-next v3 4/7] bpf, arm64: Impelment bpf_arch_text_poke() for arm64
From: Xu Kuohai @ 2022-04-24 15:40 UTC (permalink / raw)
  To: bpf, linux-arm-kernel, linux-kernel, netdev, linux-kselftest
  Cc: Catalin Marinas, Will Deacon, Steven Rostedt, Ingo Molnar,
	Daniel Borkmann, Alexei Starovoitov, Zi Shen Lim, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, David S . Miller, Hideaki YOSHIFUJI, David Ahern,
	Thomas Gleixner, Borislav Petkov, Dave Hansen, x86, hpa,
	Shuah Khan, Jakub Kicinski, Jesper Dangaard Brouer, Mark Rutland,
	Pasha Tatashin, Ard Biesheuvel, Daniel Kiss, Steven Price,
	Sudeep Holla, Marc Zyngier, Peter Collingbourne, Mark Brown,
	Delyan Kratunov, Kumar Kartikeya Dwivedi
In-Reply-To: <20220424154028.1698685-1-xukuohai@huawei.com>

Impelment bpf_arch_text_poke() for arm64, so bpf trampoline code can use
it to replace nop with jump, or replace jump with nop.

Signed-off-by: Xu Kuohai <xukuohai@huawei.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
 arch/arm64/net/bpf_jit_comp.c | 63 +++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 8ab4035dea27..3f9bdfec54c4 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -9,6 +9,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bpf.h>
+#include <linux/memory.h>
 #include <linux/filter.h>
 #include <linux/printk.h>
 #include <linux/slab.h>
@@ -18,6 +19,7 @@
 #include <asm/cacheflush.h>
 #include <asm/debug-monitors.h>
 #include <asm/insn.h>
+#include <asm/patching.h>
 #include <asm/set_memory.h>
 
 #include "bpf_jit.h"
@@ -1529,3 +1531,64 @@ void bpf_jit_free_exec(void *addr)
 {
 	return vfree(addr);
 }
+
+static int gen_branch_or_nop(enum aarch64_insn_branch_type type, void *ip,
+			     void *addr, u32 *insn)
+{
+	if (!addr)
+		*insn = aarch64_insn_gen_nop();
+	else
+		*insn = aarch64_insn_gen_branch_imm((unsigned long)ip,
+						    (unsigned long)addr,
+						    type);
+
+	return *insn != AARCH64_BREAK_FAULT ? 0 : -EFAULT;
+}
+
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type,
+		       void *old_addr, void *new_addr)
+{
+	int ret;
+	u32 old_insn;
+	u32 new_insn;
+	u32 replaced;
+	enum aarch64_insn_branch_type branch_type;
+
+	if (!is_bpf_text_address((long)ip))
+		/* Only poking bpf text is supported. Since kernel function
+		 * entry is set up by ftrace, we reply on ftrace to poke kernel
+		 * functions. For kernel funcitons, bpf_arch_text_poke() is only
+		 * called after a failed poke with ftrace. In this case, there
+		 * is probably something wrong with fentry, so there is nothing
+		 * we can do here. See register_fentry, unregister_fentry and
+		 * modify_fentry for details.
+		 */
+		return -EINVAL;
+
+	if (poke_type == BPF_MOD_CALL)
+		branch_type = AARCH64_INSN_BRANCH_LINK;
+	else
+		branch_type = AARCH64_INSN_BRANCH_NOLINK;
+
+	if (gen_branch_or_nop(branch_type, ip, old_addr, &old_insn) < 0)
+		return -EFAULT;
+
+	if (gen_branch_or_nop(branch_type, ip, new_addr, &new_insn) < 0)
+		return -EFAULT;
+
+	mutex_lock(&text_mutex);
+	if (aarch64_insn_read(ip, &replaced)) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	if (replaced != old_insn) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	ret = aarch64_insn_patch_text_nosync((void *)ip, new_insn);
+out:
+	mutex_unlock(&text_mutex);
+	return ret;
+}
-- 
2.30.2


^ permalink raw reply related

* Re: [PATCH iproute2-next 1/3] libbpf: Use bpf_object__load instead of bpf_object__load_xattr
From: David Ahern @ 2022-04-24 16:10 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, stephen, toke, Paul Chaignon
In-Reply-To: <YmSuaX7MUIqoNbC3@Laptop-X1>

On 4/23/22 7:56 PM, Hangbin Liu wrote:
> Hi David,
> 
> This patch revert c04e45d0 lib/bpf: fix verbose flag when using libbpf,
> Should we set prog->log_level directly before it loaded, like
> bpf_program__set_log_level() does?
> 

that API is new - Dec 2021 so it will not be present across relevant
libbpf versions. Detecting what exists in a libbpf version and adding
compat wrappers needs to be added. That's an undertaking I do not have
time for at the moment. If you or someone else does it would be appreciated.

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
From: Julia Lawall @ 2022-04-24 16:15 UTC (permalink / raw)
  To: Alaa Mohamed
  Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
	anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
	claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
	intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
	bridge
In-Reply-To: <0d09ad611bb78b9113491007955f2211f3a06e82.1650800975.git.eng.alaamohamedsoliman.am@gmail.com>



On Sun, 24 Apr 2022, Alaa Mohamed wrote:

> Add extack to vxlan_fdb_delete and vxlan_fdb_parse

It could be helpful to say more about what adding extack support involves.

>
> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> ---
> changes in V2:
> 	- fix spelling vxlan_fdb_delete
> 	- add missing braces
> 	- edit error message
> ---
> changes in V3:
> 	fix errors reported by checkpatch.pl

But your changes would seem to also be introducing more checkpatch errors,
because the Linux kernel coding style puts a space before an { at the
start of an if branch.

julia

> ---
>  drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index cf2f60037340..4e1886655101 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f,
>
>  static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>  			   union vxlan_addr *ip, __be16 *port, __be32 *src_vni,
> -			   __be32 *vni, u32 *ifindex, u32 *nhid)
> +			   __be32 *vni, u32 *ifindex, u32 *nhid, struct netlink_ext_ack *extack)
>  {
>  	struct net *net = dev_net(vxlan->dev);
>  	int err;
>
>  	if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
> -	    tb[NDA_PORT]))
> -		return -EINVAL;
> +	    tb[NDA_PORT])){
> +			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
> +			return -EINVAL;
> +		}
>
>  	if (tb[NDA_DST]) {
>  		err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
> -		if (err)
> +		if (err){
> +			NL_SET_ERR_MSG(extack, "Unsupported address family");
>  			return err;
> +		}
>  	} else {
>  		union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
>
> @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>  	}
>
>  	if (tb[NDA_PORT]) {
> -		if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
> +		if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
> +			NL_SET_ERR_MSG(extack, "Invalid vxlan port");
>  			return -EINVAL;
> +		}
>  		*port = nla_get_be16(tb[NDA_PORT]);
>  	} else {
>  		*port = vxlan->cfg.dst_port;
>  	}
>
>  	if (tb[NDA_VNI]) {
> -		if (nla_len(tb[NDA_VNI]) != sizeof(u32))
> +		if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
> +			NL_SET_ERR_MSG(extack, "Invalid vni");
>  			return -EINVAL;
> +		}
>  		*vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
>  	} else {
>  		*vni = vxlan->default_dst.remote_vni;
>  	}
>
>  	if (tb[NDA_SRC_VNI]) {
> -		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
> +		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
> +			NL_SET_ERR_MSG(extack, "Invalid src vni");
>  			return -EINVAL;
> +		}
>  		*src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
>  	} else {
>  		*src_vni = vxlan->default_dst.remote_vni;
> @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>  	if (tb[NDA_IFINDEX]) {
>  		struct net_device *tdev;
>
> -		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
> +		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
> +			NL_SET_ERR_MSG(extack, "Invalid ifindex");
>  			return -EINVAL;
> +		}
>  		*ifindex = nla_get_u32(tb[NDA_IFINDEX]);
>  		tdev = __dev_get_by_index(net, *ifindex);
> -		if (!tdev)
> +		if (!tdev){
> +			NL_SET_ERR_MSG(extack,"Device not found");
>  			return -EADDRNOTAVAIL;
> +		}
>  	} else {
>  		*ifindex = 0;
>  	}
> @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>  		return -EINVAL;
>
>  	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> -			      &nhid);
> +			      &nhid, extack);
>  	if (err)
>  		return err;
>
> @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>  	int err;
>
>  	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> -			      &nhid);
> +			      &nhid, extack);
>  	if (err)
>  		return err;
>
> --
> 2.36.0
>
>
>

^ permalink raw reply

* Re: [PATCH net-next v2] selftests: net: vrf_strict_mode_test: add support to select a test to run
From: David Ahern @ 2022-04-24 16:29 UTC (permalink / raw)
  To: Roopa Prabhu, Jaehee Park, outreachy, Julia Denham, Roopa Prabhu,
	Stefano Brivio, netdev
In-Reply-To: <c2eb6a8a-531e-7a6e-267c-23577f2e95e8@nvidia.com>

On 4/23/22 9:48 PM, Roopa Prabhu wrote:
> 
> On 4/21/22 09:40, Jaehee Park wrote:
>> Add a boilerplate test loop to run all tests in
>> vrf_strict_mode_test.sh. Add a -t flag that allows a selected test to
>> run.
>>
>> Signed-off-by: Jaehee Park <jhpark1013@gmail.com>
>> ---
> 
> Thanks Jaehee.
> 
> CC, David Ahern
> 
> David, this might be an overkill for this test. But nonetheless a step
> towards bringing some uniformity in the tests.
> 
> next step is to ideally move this to a library to remove repeating this
> boilerplate loop in every test.
> 
> 
> .../selftests/net/vrf_strict_mode_test.sh | 31 ++++++++++++++++++-
> 
>>   1 file changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/net/vrf_strict_mode_test.sh
>> b/tools/testing/selftests/net/vrf_strict_mode_test.sh
>> index 865d53c1781c..ca4379265706 100755
>> --- a/tools/testing/selftests/net/vrf_strict_mode_test.sh
>> +++ b/tools/testing/selftests/net/vrf_strict_mode_test.sh
>> @@ -14,6 +14,8 @@ INIT_NETNS_NAME="init"
>>     PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
>>   +TESTS="init testns mix"
>> +
>>   log_test()
>>   {
>>       local rc=$1
>> @@ -353,6 +355,23 @@ vrf_strict_mode_tests()
>>       vrf_strict_mode_tests_mix
>>   }

Add:

################################################################################
# usage

>>   +usage()
>> +{
>> +    cat <<EOF
>> +usage: ${0##*/} OPTS
>> +
>> +    -t <test> Test(s) to run (default: all)
>> +          (options: $TESTS)
>> +EOF
>> +}

Add:

################################################################################
# main

>> +while getopts ":t:h" opt; do
>> +    case $opt in
>> +        t) TESTS=$OPTARG;;
>> +        h) usage; exit 0;;
>> +        *) usage; exit 1;;
>> +    esac
>> +done
>> +
>>   vrf_strict_mode_check_support()
>>   {
>>       local nsname=$1
>> @@ -391,7 +410,17 @@ fi
>>   cleanup &> /dev/null
>>     setup
>> -vrf_strict_mode_tests
>> +for t in $TESTS
>> +do
>> +    case $t in
>> +    vrf_strict_mode_tests_init|init) vrf_strict_mode_tests_init;;
>> +    vrf_strict_mode_tests_testns|testns) vrf_strict_mode_tests_testns;;
>> +    vrf_strict_mode_tests_mix|mix) vrf_strict_mode_tests_mix;;
>> +
>> +    help) echo "Test names: $TESTS"; exit 0;;
>> +
>> +    esac
>> +done
>>   cleanup
>>     print_log_test_results

This change makes vrf_strict_mode_tests unused. Move the log_section
before the tests in that function to the function handling the test.
e.g., move 'log_section "VRF strict_mode test on init network
namespace"' to vrf_strict_mode_tests_init.

Alsom, make sure you are using tabs for indentation vs spaces.

^ permalink raw reply

* [PATCH v1 0/4] can: ctucanfd: clenup acoording to the actual rules and documentation linking
From: Pavel Pisa @ 2022-04-24 16:28 UTC (permalink / raw)
  To: linux-can, Marc Kleine-Budde, Oliver Hartkopp
  Cc: Wolfgang Grandegger, David Miller, netdev, linux-kernel,
	Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
	Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
	Matej Vasilevski, Pavel Pisa

The minor problems reported by actual checkpatch.pl and patchwork
rules has been resolved at price of disable of some debugging
options used initially to locate FPGA PCIe integration problems.

The CTU CAN FD IP core driver documentation has been linked
into CAN drivers index.

The code has been tested on QEMU with CTU CAN FD IP core
included functional model of PCIe integration.

The Linux net-next CTU CAN FD driver has been tested on real Xilinx
Zynq hardware by Matej Vasilevski even together with his
timestamp support patches. Preparation for public discussion
and mainlining is work in progress.

Jiapeng Chong (2):
  can: ctucanfd: Remove unnecessary print function dev_err()
  can: ctucanfd: Remove unused including <linux/version.h>

Pavel Pisa (2):
  can: ctucanfd: remove PCI module debug parameters and core debug
    statements.
  docs: networking: device drivers: can: add ctucanfd and its author
    e-mail update

 .../can/ctu/ctucanfd-driver.rst               |  2 +-
 .../networking/device_drivers/can/index.rst   |  1 +
 drivers/net/can/ctucanfd/ctucanfd_base.c      | 34 ++-----------------
 drivers/net/can/ctucanfd/ctucanfd_pci.c       | 22 ++++--------
 drivers/net/can/ctucanfd/ctucanfd_platform.c  |  1 -
 5 files changed, 11 insertions(+), 49 deletions(-)

-- 
2.20.1



^ permalink raw reply

* [PATCH v1 1/4] can: ctucanfd: remove PCI module debug parameters and core debug statements
From: Pavel Pisa @ 2022-04-24 16:28 UTC (permalink / raw)
  To: linux-can, Marc Kleine-Budde, Oliver Hartkopp
  Cc: Wolfgang Grandegger, David Miller, netdev, linux-kernel,
	Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
	Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
	Matej Vasilevski, Pavel Pisa
In-Reply-To: <cover.1650816929.git.pisa@cmp.felk.cvut.cz>

This and remove of inline keyword from the local static functions
should make happy all checks in actual versions of the both checkpatch.pl
and patchwork tools.

Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
---
 drivers/net/can/ctucanfd/ctucanfd_base.c | 33 +++---------------------
 drivers/net/can/ctucanfd/ctucanfd_pci.c  | 22 +++++-----------
 2 files changed, 9 insertions(+), 46 deletions(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
index 7a4550f60abb..a1f6d37fca11 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -133,13 +133,12 @@ static u32 ctucan_read32_be(struct ctucan_priv *priv,
 	return ioread32be(priv->mem_base + reg);
 }
 
-static inline void ctucan_write32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg,
-				  u32 val)
+static void ctucan_write32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg, u32 val)
 {
 	priv->write_reg(priv, reg, val);
 }
 
-static inline u32 ctucan_read32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg)
+static u32 ctucan_read32(struct ctucan_priv *priv, enum ctu_can_fd_can_registers reg)
 {
 	return priv->read_reg(priv, reg);
 }
@@ -179,8 +178,6 @@ static int ctucan_reset(struct net_device *ndev)
 	struct ctucan_priv *priv = netdev_priv(ndev);
 	int i = 100;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	ctucan_write32(priv, CTUCANFD_MODE, REG_MODE_RST);
 	clear_bit(CTUCANFD_FLAG_RX_FFW_BUFFERED, &priv->drv_flags);
 
@@ -266,8 +263,6 @@ static int ctucan_set_bittiming(struct net_device *ndev)
 	struct ctucan_priv *priv = netdev_priv(ndev);
 	struct can_bittiming *bt = &priv->can.bittiming;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	/* Note that bt may be modified here */
 	return ctucan_set_btr(ndev, bt, true);
 }
@@ -283,8 +278,6 @@ static int ctucan_set_data_bittiming(struct net_device *ndev)
 	struct ctucan_priv *priv = netdev_priv(ndev);
 	struct can_bittiming *dbt = &priv->can.data_bittiming;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	/* Note that dbt may be modified here */
 	return ctucan_set_btr(ndev, dbt, false);
 }
@@ -302,8 +295,6 @@ static int ctucan_set_secondary_sample_point(struct net_device *ndev)
 	int ssp_offset = 0;
 	u32 ssp_cfg = 0; /* No SSP by default */
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	if (CTU_CAN_FD_ENABLED(priv)) {
 		netdev_err(ndev, "BUG! Cannot set SSP - CAN is enabled\n");
 		return -EPERM;
@@ -390,8 +381,6 @@ static int ctucan_chip_start(struct net_device *ndev)
 	int err;
 	struct can_ctrlmode mode;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	priv->txb_prio = 0x01234567;
 	priv->txb_head = 0;
 	priv->txb_tail = 0;
@@ -457,8 +446,6 @@ static int ctucan_do_set_mode(struct net_device *ndev, enum can_mode mode)
 {
 	int ret;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	switch (mode) {
 	case CAN_MODE_START:
 		ret = ctucan_reset(ndev);
@@ -486,7 +473,7 @@ static int ctucan_do_set_mode(struct net_device *ndev, enum can_mode mode)
  *
  * Return: Status of TXT buffer
  */
-static inline enum ctucan_txtb_status ctucan_get_tx_status(struct ctucan_priv *priv, u8 buf)
+static enum ctucan_txtb_status ctucan_get_tx_status(struct ctucan_priv *priv, u8 buf)
 {
 	u32 tx_status = ctucan_read32(priv, CTUCANFD_TX_STATUS);
 	enum ctucan_txtb_status status = (tx_status >> (buf * 4)) & 0x7;
@@ -1123,8 +1110,6 @@ static irqreturn_t ctucan_interrupt(int irq, void *dev_id)
 	u32 imask;
 	int irq_loops;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	for (irq_loops = 0; irq_loops < 10000; irq_loops++) {
 		/* Get the interrupt status */
 		isr = ctucan_read32(priv, CTUCANFD_INT_STAT);
@@ -1198,8 +1183,6 @@ static void ctucan_chip_stop(struct net_device *ndev)
 	u32 mask = 0xffffffff;
 	u32 mode;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	/* Disable interrupts and disable CAN */
 	ctucan_write32(priv, CTUCANFD_INT_ENA_CLR, mask);
 	ctucan_write32(priv, CTUCANFD_INT_MASK_SET, mask);
@@ -1222,8 +1205,6 @@ static int ctucan_open(struct net_device *ndev)
 	struct ctucan_priv *priv = netdev_priv(ndev);
 	int ret;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	ret = pm_runtime_get_sync(priv->dev);
 	if (ret < 0) {
 		netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
@@ -1283,8 +1264,6 @@ static int ctucan_close(struct net_device *ndev)
 {
 	struct ctucan_priv *priv = netdev_priv(ndev);
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	netif_stop_queue(ndev);
 	napi_disable(&priv->napi);
 	ctucan_chip_stop(ndev);
@@ -1310,8 +1289,6 @@ static int ctucan_get_berr_counter(const struct net_device *ndev, struct can_ber
 	struct ctucan_priv *priv = netdev_priv(ndev);
 	int ret;
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	ret = pm_runtime_get_sync(priv->dev);
 	if (ret < 0) {
 		netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n", __func__, ret);
@@ -1337,8 +1314,6 @@ int ctucan_suspend(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct ctucan_priv *priv = netdev_priv(ndev);
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	if (netif_running(ndev)) {
 		netif_stop_queue(ndev);
 		netif_device_detach(ndev);
@@ -1355,8 +1330,6 @@ int ctucan_resume(struct device *dev)
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct ctucan_priv *priv = netdev_priv(ndev);
 
-	ctucan_netdev_dbg(ndev, "%s\n", __func__);
-
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	if (netif_running(ndev)) {
diff --git a/drivers/net/can/ctucanfd/ctucanfd_pci.c b/drivers/net/can/ctucanfd/ctucanfd_pci.c
index c37a42480533..8f2956a8ae43 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_pci.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_pci.c
@@ -45,14 +45,6 @@
 #define CTUCAN_WITHOUT_CTUCAN_ID  0
 #define CTUCAN_WITH_CTUCAN_ID     1
 
-static bool use_msi = true;
-module_param(use_msi, bool, 0444);
-MODULE_PARM_DESC(use_msi, "PCIe implementation use MSI interrupts. Default: 1 (yes)");
-
-static bool pci_use_second = true;
-module_param(pci_use_second, bool, 0444);
-MODULE_PARM_DESC(pci_use_second, "Use the second CAN core on PCIe card. Default: 1 (yes)");
-
 struct ctucan_pci_board_data {
 	void __iomem *bar0_base;
 	void __iomem *cra_base;
@@ -117,13 +109,11 @@ static int ctucan_pci_probe(struct pci_dev *pdev,
 		goto err_disable_device;
 	}
 
-	if (use_msi) {
-		ret = pci_enable_msi(pdev);
-		if (!ret) {
-			dev_info(dev, "MSI enabled\n");
-			pci_set_master(pdev);
-			msi_ok = 1;
-		}
+	ret = pci_enable_msi(pdev);
+	if (!ret) {
+		dev_info(dev, "MSI enabled\n");
+		pci_set_master(pdev);
+		msi_ok = 1;
 	}
 
 	dev_info(dev, "ctucan BAR0 0x%08llx 0x%08llx\n",
@@ -184,7 +174,7 @@ static int ctucan_pci_probe(struct pci_dev *pdev,
 
 	core_i++;
 
-	while (pci_use_second && (core_i < num_cores)) {
+	while (core_i < num_cores) {
 		addr += 0x4000;
 		ret = ctucan_probe_common(dev, addr, irq, ntxbufs, 100000000,
 					  0, ctucan_pci_set_drvdata);
-- 
2.20.1



^ permalink raw reply related

* [PATCH v1 3/4] can: ctucanfd: Remove unused including <linux/version.h>
From: Pavel Pisa @ 2022-04-24 16:28 UTC (permalink / raw)
  To: linux-can, Marc Kleine-Budde, Oliver Hartkopp
  Cc: Wolfgang Grandegger, David Miller, netdev, linux-kernel,
	Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
	Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
	Matej Vasilevski, Jiapeng Chong, Abaci Robot, Pavel Pisa
In-Reply-To: <cover.1650816929.git.pisa@cmp.felk.cvut.cz>

From: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

Eliminate the follow versioncheck warning:

./drivers/net/can/ctucanfd/ctucanfd_base.c: 34 linux/version.h not
needed.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
---
 drivers/net/can/ctucanfd/ctucanfd_base.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
index a1f6d37fca11..2ada097d1ede 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -31,7 +31,6 @@
 #include <linux/can/error.h>
 #include <linux/can/led.h>
 #include <linux/pm_runtime.h>
-#include <linux/version.h>
 
 #include "ctucanfd.h"
 #include "ctucanfd_kregs.h"
-- 
2.20.1



^ permalink raw reply related

* [PATCH v1 4/4] docs: networking: device drivers: can: add ctucanfd and its author e-mail update
From: Pavel Pisa @ 2022-04-24 16:28 UTC (permalink / raw)
  To: linux-can, Marc Kleine-Budde, Oliver Hartkopp
  Cc: Wolfgang Grandegger, David Miller, netdev, linux-kernel,
	Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
	Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
	Matej Vasilevski, Pavel Pisa
In-Reply-To: <cover.1650816929.git.pisa@cmp.felk.cvut.cz>

Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
---
 .../networking/device_drivers/can/ctu/ctucanfd-driver.rst       | 2 +-
 Documentation/networking/device_drivers/can/index.rst           | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst b/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
index 797fb45be187..2fde5551e756 100644
--- a/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
+++ b/Documentation/networking/device_drivers/can/ctu/ctucanfd-driver.rst
@@ -536,7 +536,7 @@ CTU CAN FD Driver Sources Reference
 CTU CAN FD IP Core and Driver Development Acknowledgment
 ---------------------------------------------------------
 
-* Odrej Ille <illeondr@fel.cvut.cz>
+* Odrej Ille <ondrej.ille@gmail.com>
 
   * started the project as student at Department of Measurement, FEE, CTU
   * invested great amount of personal time and enthusiasm to the project over years
diff --git a/Documentation/networking/device_drivers/can/index.rst b/Documentation/networking/device_drivers/can/index.rst
index 58b6e0ad3030..0c3cc6633559 100644
--- a/Documentation/networking/device_drivers/can/index.rst
+++ b/Documentation/networking/device_drivers/can/index.rst
@@ -10,6 +10,7 @@ Contents:
 .. toctree::
    :maxdepth: 2
 
+   ctu/ctucanfd-driver
    freescale/flexcan
 
 .. only::  subproject and html
-- 
2.20.1



^ permalink raw reply related

* [PATCH v1 2/4] can: ctucanfd: Remove unnecessary print function dev_err()
From: Pavel Pisa @ 2022-04-24 16:28 UTC (permalink / raw)
  To: linux-can, Marc Kleine-Budde, Oliver Hartkopp
  Cc: Wolfgang Grandegger, David Miller, netdev, linux-kernel,
	Marin Jerabek, Ondrej Ille, Jiri Novak, Jaroslav Beran,
	Petr Porazil, Pavel Machek, Carsten Emde, Drew Fustini,
	Matej Vasilevski, Jiapeng Chong, Abaci Robot, Pavel Pisa
In-Reply-To: <cover.1650816929.git.pisa@cmp.felk.cvut.cz>

From: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

The print function dev_err() is redundant because platform_get_irq()
already prints an error.

Eliminate the follow coccicheck warnings:

./drivers/net/can/ctucanfd/ctucanfd_platform.c:67:2-9: line 67 is
redundant because platform_get_irq() already prints an error.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>
---
 drivers/net/can/ctucanfd/ctucanfd_platform.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_platform.c b/drivers/net/can/ctucanfd/ctucanfd_platform.c
index 5e4806068662..89d54c2151e1 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_platform.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_platform.c
@@ -64,7 +64,6 @@ static int ctucan_platform_probe(struct platform_device *pdev)
 	}
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
-		dev_err(dev, "Cannot find interrupt.\n");
 		ret = irq;
 		goto err;
 	}
-- 
2.20.1



^ permalink raw reply related

* [PATCH net-next RESEND] net: bcmgenet: hide status block before TX timestamping
From: Jonathan Lemon @ 2022-04-24 16:53 UTC (permalink / raw)
  To: opendmb, f.fainelli, bcm-kernel-feedback-list; +Cc: netdev, kernel-team, kuba

The hardware checksum offloading requires use of a transmit
status block inserted before the outgoing frame data, this was
updated in '9a9ba2a4aaaa ("net: bcmgenet: always enable status blocks")'

However, skb_tx_timestamp() assumes that it is passed a raw frame
and PTP parsing chokes on this status block.

Fix this by calling __skb_pull(), which hides the TSB before calling
skb_tx_timestamp(), so an outgoing PTP packet is parsed correctly.

As the data in the skb has already been set up for DMA, and the
dma_unmap_* calls use a separately stored address, there is no
no effective change in the data transmission.

Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 9a41145dadfc..bf1ec8fdc2ad 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -2035,6 +2035,11 @@ static struct sk_buff *bcmgenet_add_tsb(struct net_device *dev,
 	return skb;
 }
 
+static void bcmgenet_hide_tsb(struct sk_buff *skb)
+{
+	__skb_pull(skb, sizeof(struct status_64));
+}
+
 static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct bcmgenet_priv *priv = netdev_priv(dev);
@@ -2141,6 +2146,8 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	GENET_CB(skb)->last_cb = tx_cb_ptr;
+
+	bcmgenet_hide_tsb(skb);
 	skb_tx_timestamp(skb);
 
 	/* Decrement total BD count and advance our write pointer */
-- 
2.31.1


^ permalink raw reply related

* [PATCH bpf] selftests/bpf: test setting tunnel key from lwt xmit
From: Eyal Birger @ 2022-04-24 16:53 UTC (permalink / raw)
  To: shuah, ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, posk, netdev,
	bpf, Eyal Birger

This commit adds test_egress_md() tests which perform a similar flow as
test_egress() only that they use gre devices in collect_md mode and set
the tunnel key from lwt bpf xmit.

VRF scenarios are not checked since it is currently not possible to set
the underlying device or vrf from bpf_set_tunnel_key().

This introduces minor changes to the existing setup for consistency with
the new tests:

- GRE key must exist as bpf_set_tunnel_key() explicitly sets the
  TUNNEL_KEY flag

- Source address for GRE traffic is set to IPv*_5 instead of IPv*_1 since
  GRE traffic is sent via veth5 so its address is selected when using
  bpf_set_tunnel_key()

Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
 .../selftests/bpf/progs/test_lwt_ip_encap.c   | 51 ++++++++++-
 .../selftests/bpf/test_lwt_ip_encap.sh        | 85 ++++++++++++++++++-
 2 files changed, 128 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c b/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c
index d6cb986e7533..39c6bd5402ae 100644
--- a/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c
+++ b/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c
@@ -10,8 +10,11 @@
 struct grehdr {
 	__be16 flags;
 	__be16 protocol;
+	__be32 key;
 };
 
+#define GRE_KEY	0x2000
+
 SEC("encap_gre")
 int bpf_lwt_encap_gre(struct __sk_buff *skb)
 {
@@ -28,10 +31,10 @@ int bpf_lwt_encap_gre(struct __sk_buff *skb)
 	hdr.iph.ttl = 0x40;
 	hdr.iph.protocol = 47;  /* IPPROTO_GRE */
 #if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
-	hdr.iph.saddr = 0x640110ac;  /* 172.16.1.100 */
+	hdr.iph.saddr = 0x640510ac;  /* 172.16.5.100 */
 	hdr.iph.daddr = 0x641010ac;  /* 172.16.16.100 */
 #elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
-	hdr.iph.saddr = 0xac100164;  /* 172.16.1.100 */
+	hdr.iph.saddr = 0xac100564;  /* 172.16.5.100 */
 	hdr.iph.daddr = 0xac101064;  /* 172.16.16.100 */
 #else
 #error "Fix your compiler's __BYTE_ORDER__?!"
@@ -39,6 +42,7 @@ int bpf_lwt_encap_gre(struct __sk_buff *skb)
 	hdr.iph.tot_len = bpf_htons(skb->len + sizeof(struct encap_hdr));
 
 	hdr.greh.protocol = skb->protocol;
+	hdr.greh.flags = bpf_htons(GRE_KEY);
 
 	err = bpf_lwt_push_encap(skb, BPF_LWT_ENCAP_IP, &hdr,
 				 sizeof(struct encap_hdr));
@@ -63,9 +67,9 @@ int bpf_lwt_encap_gre6(struct __sk_buff *skb)
 	hdr.ip6hdr.payload_len = bpf_htons(skb->len + sizeof(struct grehdr));
 	hdr.ip6hdr.nexthdr = 47;  /* IPPROTO_GRE */
 	hdr.ip6hdr.hop_limit = 0x40;
-	/* fb01::1 */
+	/* fb05::1 */
 	hdr.ip6hdr.saddr.s6_addr[0] = 0xfb;
-	hdr.ip6hdr.saddr.s6_addr[1] = 1;
+	hdr.ip6hdr.saddr.s6_addr[1] = 5;
 	hdr.ip6hdr.saddr.s6_addr[15] = 1;
 	/* fb10::1 */
 	hdr.ip6hdr.daddr.s6_addr[0] = 0xfb;
@@ -73,6 +77,7 @@ int bpf_lwt_encap_gre6(struct __sk_buff *skb)
 	hdr.ip6hdr.daddr.s6_addr[15] = 1;
 
 	hdr.greh.protocol = skb->protocol;
+	hdr.greh.flags = bpf_htons(GRE_KEY);
 
 	err = bpf_lwt_push_encap(skb, BPF_LWT_ENCAP_IP, &hdr,
 				 sizeof(struct encap_hdr));
@@ -82,4 +87,42 @@ int bpf_lwt_encap_gre6(struct __sk_buff *skb)
 	return BPF_LWT_REROUTE;
 }
 
+SEC("encap_gre_md")
+int bpf_lwt_encap_gre_md(struct __sk_buff *skb)
+{
+	struct bpf_tunnel_key key;
+	int err;
+
+	__builtin_memset(&key, 0x0, sizeof(key));
+	key.remote_ipv4 = 0xac101064; /* 172.16.16.100 - always in host order */
+	key.tunnel_ttl = 0x40;
+	err = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
+				     BPF_F_ZERO_CSUM_TX | BPF_F_SEQ_NUMBER);
+	if (err)
+		return BPF_DROP;
+
+	return BPF_OK;
+}
+
+SEC("encap_gre6_md")
+int bpf_lwt_encap_gre6_md(struct __sk_buff *skb)
+{
+	struct bpf_tunnel_key key;
+	int err;
+
+	__builtin_memset(&key, 0x0, sizeof(key));
+
+	/* fb10::1 */
+	key.remote_ipv6[0] = bpf_htonl(0xfb100000);
+	key.remote_ipv6[3] = bpf_htonl(0x01);
+	key.tunnel_ttl = 0x40;
+	err = bpf_skb_set_tunnel_key(skb, &key, sizeof(key),
+				     BPF_F_ZERO_CSUM_TX | BPF_F_SEQ_NUMBER |
+				     BPF_F_TUNINFO_IPV6);
+	if (err)
+		return BPF_DROP;
+
+	return BPF_OK;
+}
+
 char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_lwt_ip_encap.sh b/tools/testing/selftests/bpf/test_lwt_ip_encap.sh
index 6c69c42b1d60..86c4a172f90c 100755
--- a/tools/testing/selftests/bpf/test_lwt_ip_encap.sh
+++ b/tools/testing/selftests/bpf/test_lwt_ip_encap.sh
@@ -37,6 +37,14 @@
 #
 #       ping: SRC->[encap at veth2:ingress]->GRE:decap->DST
 #       ping replies go DST->SRC directly
+#
+#   2c. in an egress_md test, a bpf LWT_XMIT program is installed on a
+#	route towards a collect_md gre{,6} device and sets the tunnel key
+#	such that packets are encapsulated with an IP/GRE header to route
+#	to IPv*_GRE
+#
+#       ping: SRC->[encap at gre{,6}_md:xmit]->GRE:decap->DST
+#       ping replies go DST->SRC directly
 
 if [[ $EUID -ne 0 ]]; then
 	echo "This script must be run as root"
@@ -238,7 +246,7 @@ setup()
 	ip -netns ${NS3} -6 route add ${IPv6_6}/128 dev veth8 via ${IPv6_7}
 
 	# configure IPv4 GRE device in NS3, and a route to it via the "bottom" route
-	ip -netns ${NS3} tunnel add gre_dev mode gre remote ${IPv4_1} local ${IPv4_GRE} ttl 255
+	ip -netns ${NS3} tunnel add gre_dev mode gre remote ${IPv4_5} local ${IPv4_GRE} ttl 255 key 0
 	ip -netns ${NS3} link set gre_dev up
 	ip -netns ${NS3} addr add ${IPv4_GRE} dev gre_dev
 	ip -netns ${NS1} route add ${IPv4_GRE}/32 dev veth5 via ${IPv4_6} ${VRF}
@@ -246,7 +254,7 @@ setup()
 
 
 	# configure IPv6 GRE device in NS3, and a route to it via the "bottom" route
-	ip -netns ${NS3} -6 tunnel add name gre6_dev mode ip6gre remote ${IPv6_1} local ${IPv6_GRE} ttl 255
+	ip -netns ${NS3} -6 tunnel add name gre6_dev mode ip6gre remote ${IPv6_5} local ${IPv6_GRE} ttl 255 key 0
 	ip -netns ${NS3} link set gre6_dev up
 	ip -netns ${NS3} -6 addr add ${IPv6_GRE} nodad dev gre6_dev
 	ip -netns ${NS1} -6 route add ${IPv6_GRE}/128 dev veth5 via ${IPv6_6} ${VRF}
@@ -291,13 +299,16 @@ test_ping()
 {
 	local readonly PROTO=$1
 	local readonly EXPECTED=$2
+	local readonly NOBIND=$3
 	local RET=0
 
+	BINDTODEV=$([ -z ${NOBIND} ] && echo -I veth1)
+
 	if [ "${PROTO}" == "IPv4" ] ; then
-		ip netns exec ${NS1} ping  -c 1 -W 1 -I veth1 ${IPv4_DST} 2>&1 > /dev/null
+		ip netns exec ${NS1} ping  -c 1 -W 1 ${BINDTODEV} ${IPv4_DST} 2>&1 > /dev/null
 		RET=$?
 	elif [ "${PROTO}" == "IPv6" ] ; then
-		ip netns exec ${NS1} ping6 -c 1 -W 1 -I veth1 ${IPv6_DST} 2>&1 > /dev/null
+		ip netns exec ${NS1} ping6 -c 1 -W 1 ${BINDTODEV} ${IPv6_DST} 2>&1 > /dev/null
 		RET=$?
 	else
 		echo "    test_ping: unknown PROTO: ${PROTO}"
@@ -409,6 +420,70 @@ test_egress()
 	process_test_results
 }
 
+test_egress_md()
+{
+	local readonly ENCAP=$1
+	echo "starting egress_md ${ENCAP} encap test"
+	setup
+
+	# by default, pings work
+	test_ping IPv4 0
+	test_ping IPv6 0
+
+	# remove NS2->DST routes, ping fails
+	ip -netns ${NS2}    route del ${IPv4_DST}/32  dev veth3
+	ip -netns ${NS2} -6 route del ${IPv6_DST}/128 dev veth3
+	test_ping IPv4 1
+	test_ping IPv6 1
+
+	# install replacement routes (LWT/eBPF), pings succeed
+	if [ "${ENCAP}" == "IPv4" ] ; then
+		ip -net ${NS1} link add gre_md type gre external
+		ip -netns ${NS1}    addr add ${IPv4_1}/24  dev gre_md
+		ip -netns ${NS1} -6 addr add ${IPv6_1}/128 nodad dev gre_md
+		ip -netns ${NS1} link set gre_md up
+
+		ip -netns ${NS1} route add ${IPv4_DST} encap bpf xmit obj \
+			test_lwt_ip_encap.o sec encap_gre_md dev gre_md
+		ip -netns ${NS1} -6 route add ${IPv6_DST} encap bpf xmit obj \
+			test_lwt_ip_encap.o sec encap_gre_md dev gre_md
+	elif [ "${ENCAP}" == "IPv6" ] ; then
+		ip -net ${NS1} link add gre6_md type ip6gre external
+		ip -netns ${NS1}    addr add ${IPv4_1}/24  dev gre6_md
+		ip -netns ${NS1} -6 addr add ${IPv6_1}/128 nodad dev gre6_md
+		ip -netns ${NS1} link set gre6_md up
+
+		ip -netns ${NS1} route add ${IPv4_DST} encap bpf xmit obj \
+			test_lwt_ip_encap.o sec encap_gre6_md dev gre6_md
+		ip -netns ${NS1} -6 route add ${IPv6_DST} encap bpf xmit obj \
+			test_lwt_ip_encap.o sec encap_gre6_md dev gre6_md
+	else
+		echo "    unknown encap ${ENCAP}"
+		TEST_STATUS=1
+	fi
+
+	# Due to the asymmetry of the traffic flow we do not bind to device
+
+	test_ping IPv4 0 nobind
+	test_ping IPv6 0 nobind
+
+	test_gso IPv4
+	test_gso IPv6
+
+	# a negative test: remove routes to GRE devices: ping fails
+	remove_routes_to_gredev
+	test_ping IPv4 1 nobind
+	test_ping IPv6 1 nobind
+
+	# another negative test
+	add_unreachable_routes_to_gredev
+	test_ping IPv4 1 nobind
+	test_ping IPv6 1 nobind
+
+	cleanup
+	process_test_results
+}
+
 test_ingress()
 {
 	local readonly ENCAP=$1
@@ -465,6 +540,8 @@ test_egress IPv4
 test_egress IPv6
 test_ingress IPv4
 test_ingress IPv6
+test_egress_md IPv4
+test_egress_md IPv6
 
 VRF="vrf red"
 test_egress IPv4
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v2] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Marek Behún @ 2022-04-24 17:20 UTC (permalink / raw)
  To: Nathan Rossi
  Cc: netdev, linux-kernel, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller,
	Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220424153143.323338-1-nathan@nathanrossi.com>

On Sun, 24 Apr 2022 15:31:43 +0000
Nathan Rossi <nathan@nathanrossi.com> wrote:

> The other port_hidden functions rely on the port_read/port_write
> functions to access the hidden control port. These functions apply the
> offset for port_base_addr where applicable. Update port_hidden_wait to
> use the port_wait_bit so that port_base_addr offsets are accounted for
> when waiting for the busy bit to change.
> 
> Without the offset the port_hidden_wait function would timeout on
> devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> devices that have a zero port_base_addr would operate correctly (e.g.
> MV88E6390).
> 
> Fixes: ea89098ef9a5 ("net: dsa: mv88x6xxx: mv88e6390 errata")
> Signed-off-by: Nathan Rossi <nathan@nathanrossi.com>
> ---
> Changes in v2:
> - Add fixes
> ---
>  drivers/net/dsa/mv88e6xxx/port_hidden.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/port_hidden.c b/drivers/net/dsa/mv88e6xxx/port_hidden.c
> index b49d05f0e1..7a9f9ff6de 100644
> --- a/drivers/net/dsa/mv88e6xxx/port_hidden.c
> +++ b/drivers/net/dsa/mv88e6xxx/port_hidden.c
> @@ -40,8 +40,9 @@ int mv88e6xxx_port_hidden_wait(struct mv88e6xxx_chip *chip)
>  {
>  	int bit = __bf_shf(MV88E6XXX_PORT_RESERVED_1A_BUSY);
>  
> -	return mv88e6xxx_wait_bit(chip, MV88E6XXX_PORT_RESERVED_1A_CTRL_PORT,
> -				  MV88E6XXX_PORT_RESERVED_1A, bit, 0);
> +	return mv88e6xxx_port_wait_bit(chip,
> +				       MV88E6XXX_PORT_RESERVED_1A_CTRL_PORT,
> +				       MV88E6XXX_PORT_RESERVED_1A, bit, 0);
>  }
>  
>  int mv88e6xxx_port_hidden_read(struct mv88e6xxx_chip *chip, int block, int port,

Reviewed-by: Marek Behún <kabel@kernel.org>

^ permalink raw reply

* Re: [PATCH bpf] selftests/bpf: test setting tunnel key from lwt xmit
From: Eyal Birger @ 2022-04-24 17:23 UTC (permalink / raw)
  To: shuah, ast, daniel, andrii
  Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, posk, netdev,
	bpf
In-Reply-To: <20220424165309.241796-1-eyal.birger@gmail.com>

On Sun, Apr 24, 2022 at 7:53 PM Eyal Birger <eyal.birger@gmail.com> wrote:
>
> This commit adds test_egress_md() tests which perform a similar flow as
> test_egress() only that they use gre devices in collect_md mode and set
> the tunnel key from lwt bpf xmit.
>
> VRF scenarios are not checked since it is currently not possible to set
> the underlying device or vrf from bpf_set_tunnel_key().
>
> This introduces minor changes to the existing setup for consistency with
> the new tests:
>
> - GRE key must exist as bpf_set_tunnel_key() explicitly sets the
>   TUNNEL_KEY flag
>
> - Source address for GRE traffic is set to IPv*_5 instead of IPv*_1 since
>   GRE traffic is sent via veth5 so its address is selected when using
>   bpf_set_tunnel_key()
>
> Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
> ---
>  .../selftests/bpf/progs/test_lwt_ip_encap.c   | 51 ++++++++++-
>  .../selftests/bpf/test_lwt_ip_encap.sh        | 85 ++++++++++++++++++-
>  2 files changed, 128 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c b/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c
> index d6cb986e7533..39c6bd5402ae 100644
> --- a/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c
> +++ b/tools/testing/selftests/bpf/progs/test_lwt_ip_encap.c

Thinking about this some more, I'm not sure if these tests fit better here
or in test_tunnel.sh.

If the latter is preferred, please drop this patch and I'll submit one for
test_tunnel.sh.

Eyal.

^ permalink raw reply

* Re: [PATCH net-next v2 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
From: Stephen Hemminger @ 2022-04-24 17:39 UTC (permalink / raw)
  To: Alaa Mohamed; +Cc: netdev, outreachy, roopa, roopa.prabhu, jdenham, sbrivio
In-Reply-To: <22f5fb1d5e592c0deefb246225f66908947a613b.1650754231.git.eng.alaamohamedsoliman.am@gmail.com>

On Sun, 24 Apr 2022 00:54:08 +0200
Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com> wrote:

> +			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
> +			return -EINVAL;

why not break the line after "NL_SET_ERR_MSG(extack,"  so it is no so long?

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
From: Alaa Mohamed @ 2022-04-24 18:52 UTC (permalink / raw)
  To: Julia Lawall
  Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
	anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
	claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
	intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
	bridge
In-Reply-To: <alpine.DEB.2.22.394.2204241813210.7691@hadrien>


On ٢٤‏/٤‏/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>
> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
> It could be helpful to say more about what adding extack support involves.
>
>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>> ---
>> changes in V2:
>> 	- fix spelling vxlan_fdb_delete
>> 	- add missing braces
>> 	- edit error message
>> ---
>> changes in V3:
>> 	fix errors reported by checkpatch.pl
> But your changes would seem to also be introducing more checkpatch errors,
> because the Linux kernel coding style puts a space before an { at the
> start of an if branch.
I ran checkpatch script before submitting this patch and got no error
>
> julia
>
>> ---
>>   drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
>>   1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
>> index cf2f60037340..4e1886655101 100644
>> --- a/drivers/net/vxlan/vxlan_core.c
>> +++ b/drivers/net/vxlan/vxlan_core.c
>> @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f,
>>
>>   static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>>   			   union vxlan_addr *ip, __be16 *port, __be32 *src_vni,
>> -			   __be32 *vni, u32 *ifindex, u32 *nhid)
>> +			   __be32 *vni, u32 *ifindex, u32 *nhid, struct netlink_ext_ack *extack)
>>   {
>>   	struct net *net = dev_net(vxlan->dev);
>>   	int err;
>>
>>   	if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
>> -	    tb[NDA_PORT]))
>> -		return -EINVAL;
>> +	    tb[NDA_PORT])){
>> +			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
>> +			return -EINVAL;
>> +		}
>>
>>   	if (tb[NDA_DST]) {
>>   		err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
>> -		if (err)
>> +		if (err){
>> +			NL_SET_ERR_MSG(extack, "Unsupported address family");
>>   			return err;
>> +		}
>>   	} else {
>>   		union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
>>
>> @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>>   	}
>>
>>   	if (tb[NDA_PORT]) {
>> -		if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
>> +		if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
>> +			NL_SET_ERR_MSG(extack, "Invalid vxlan port");
>>   			return -EINVAL;
>> +		}
>>   		*port = nla_get_be16(tb[NDA_PORT]);
>>   	} else {
>>   		*port = vxlan->cfg.dst_port;
>>   	}
>>
>>   	if (tb[NDA_VNI]) {
>> -		if (nla_len(tb[NDA_VNI]) != sizeof(u32))
>> +		if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
>> +			NL_SET_ERR_MSG(extack, "Invalid vni");
>>   			return -EINVAL;
>> +		}
>>   		*vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
>>   	} else {
>>   		*vni = vxlan->default_dst.remote_vni;
>>   	}
>>
>>   	if (tb[NDA_SRC_VNI]) {
>> -		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
>> +		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
>> +			NL_SET_ERR_MSG(extack, "Invalid src vni");
>>   			return -EINVAL;
>> +		}
>>   		*src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
>>   	} else {
>>   		*src_vni = vxlan->default_dst.remote_vni;
>> @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>>   	if (tb[NDA_IFINDEX]) {
>>   		struct net_device *tdev;
>>
>> -		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
>> +		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
>> +			NL_SET_ERR_MSG(extack, "Invalid ifindex");
>>   			return -EINVAL;
>> +		}
>>   		*ifindex = nla_get_u32(tb[NDA_IFINDEX]);
>>   		tdev = __dev_get_by_index(net, *ifindex);
>> -		if (!tdev)
>> +		if (!tdev){
>> +			NL_SET_ERR_MSG(extack,"Device not found");
>>   			return -EADDRNOTAVAIL;
>> +		}
>>   	} else {
>>   		*ifindex = 0;
>>   	}
>> @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
>>   		return -EINVAL;
>>
>>   	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>> -			      &nhid);
>> +			      &nhid, extack);
>>   	if (err)
>>   		return err;
>>
>> @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>>   	int err;
>>
>>   	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>> -			      &nhid);
>> +			      &nhid, extack);
>>   	if (err)
>>   		return err;
>>
>> --
>> 2.36.0
>>
>>
>>

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
From: Julia Lawall @ 2022-04-24 18:56 UTC (permalink / raw)
  To: Alaa Mohamed
  Cc: Julia Lawall, netdev, outreachy, roopa, jdenham, sbrivio,
	jesse.brandeburg, anthony.l.nguyen, davem, kuba, pabeni,
	vladimir.oltean, claudiu.manoil, alexandre.belloni, shshaikh,
	manishc, razor, intel-wired-lan, linux-kernel, UNGLinuxDriver,
	GR-Linux-NIC-Dev, bridge
In-Reply-To: <06622e4c-b9a5-1c4f-2764-a72733000b4e@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5072 bytes --]



On Sun, 24 Apr 2022, Alaa Mohamed wrote:

>
> On ٢٤/٤/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
> >
> > On Sun, 24 Apr 2022, Alaa Mohamed wrote:
> >
> > > Add extack to vxlan_fdb_delete and vxlan_fdb_parse
> > It could be helpful to say more about what adding extack support involves.
> >
> > > Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> > > ---
> > > changes in V2:
> > > 	- fix spelling vxlan_fdb_delete
> > > 	- add missing braces
> > > 	- edit error message
> > > ---
> > > changes in V3:
> > > 	fix errors reported by checkpatch.pl
> > But your changes would seem to also be introducing more checkpatch errors,
> > because the Linux kernel coding style puts a space before an { at the
> > start of an if branch.
> I ran checkpatch script before submitting this patch and got no error

OK, whether checkpatch complains or not doesn't really matter.  The point
is that in the Linux kernel, one writes:

if (...) {

and not

if (...){

You can see other examples of ifs in the Linux kernel.

julia

> >
> > julia
> >
> > > ---
> > >   drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
> > >   1 file changed, 25 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/net/vxlan/vxlan_core.c
> > > b/drivers/net/vxlan/vxlan_core.c
> > > index cf2f60037340..4e1886655101 100644
> > > --- a/drivers/net/vxlan/vxlan_core.c
> > > +++ b/drivers/net/vxlan/vxlan_core.c
> > > @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev
> > > *vxlan, struct vxlan_fdb *f,
> > >
> > >   static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
> > >   			   union vxlan_addr *ip, __be16 *port, __be32
> > > *src_vni,
> > > -			   __be32 *vni, u32 *ifindex, u32 *nhid)
> > > +			   __be32 *vni, u32 *ifindex, u32 *nhid, struct
> > > netlink_ext_ack *extack)
> > >   {
> > >   	struct net *net = dev_net(vxlan->dev);
> > >   	int err;
> > >
> > >   	if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
> > > -	    tb[NDA_PORT]))
> > > -		return -EINVAL;
> > > +	    tb[NDA_PORT])){
> > > +			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are
> > > mutually exclusive with NH_ID");
> > > +			return -EINVAL;
> > > +		}
> > >
> > >   	if (tb[NDA_DST]) {
> > >   		err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
> > > -		if (err)
> > > +		if (err){
> > > +			NL_SET_ERR_MSG(extack, "Unsupported address family");
> > >   			return err;
> > > +		}
> > >   	} else {
> > >   		union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
> > >
> > > @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
> > > struct vxlan_dev *vxlan,
> > >   	}
> > >
> > >   	if (tb[NDA_PORT]) {
> > > -		if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
> > > +		if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
> > > +			NL_SET_ERR_MSG(extack, "Invalid vxlan port");
> > >   			return -EINVAL;
> > > +		}
> > >   		*port = nla_get_be16(tb[NDA_PORT]);
> > >   	} else {
> > >   		*port = vxlan->cfg.dst_port;
> > >   	}
> > >
> > >   	if (tb[NDA_VNI]) {
> > > -		if (nla_len(tb[NDA_VNI]) != sizeof(u32))
> > > +		if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
> > > +			NL_SET_ERR_MSG(extack, "Invalid vni");
> > >   			return -EINVAL;
> > > +		}
> > >   		*vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
> > >   	} else {
> > >   		*vni = vxlan->default_dst.remote_vni;
> > >   	}
> > >
> > >   	if (tb[NDA_SRC_VNI]) {
> > > -		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
> > > +		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
> > > +			NL_SET_ERR_MSG(extack, "Invalid src vni");
> > >   			return -EINVAL;
> > > +		}
> > >   		*src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
> > >   	} else {
> > >   		*src_vni = vxlan->default_dst.remote_vni;
> > > @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
> > > struct vxlan_dev *vxlan,
> > >   	if (tb[NDA_IFINDEX]) {
> > >   		struct net_device *tdev;
> > >
> > > -		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
> > > +		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
> > > +			NL_SET_ERR_MSG(extack, "Invalid ifindex");
> > >   			return -EINVAL;
> > > +		}
> > >   		*ifindex = nla_get_u32(tb[NDA_IFINDEX]);
> > >   		tdev = __dev_get_by_index(net, *ifindex);
> > > -		if (!tdev)
> > > +		if (!tdev){
> > > +			NL_SET_ERR_MSG(extack,"Device not found");
> > >   			return -EADDRNOTAVAIL;
> > > +		}
> > >   	} else {
> > >   		*ifindex = 0;
> > >   	}
> > > @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct
> > > nlattr *tb[],
> > >   		return -EINVAL;
> > >
> > >   	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> > > -			      &nhid);
> > > +			      &nhid, extack);
> > >   	if (err)
> > >   		return err;
> > >
> > > @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm,
> > > struct nlattr *tb[],
> > >   	int err;
> > >
> > >   	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
> > > -			      &nhid);
> > > +			      &nhid, extack);
> > >   	if (err)
> > >   		return err;
> > >
> > > --
> > > 2.36.0
> > >
> > >
> > >
>

^ permalink raw reply

* re: ctcm: rename READ/WRITE defines to avoid redefinitions
From: Colin King (gmail) @ 2022-04-24 18:58 UTC (permalink / raw)
  To: Alexandra Winter, Wenjia Zhang, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev
  Cc: Christian Borntraeger, linux-s390, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

Hi,

static analysis with cppcheck detected a potential null pointer 
deference with the following commit:

commit 3c09e2647b5e1f1f9fd383971468823c2505e1b0
Author: Ursula Braun <ursula.braun@de.ibm.com>
Date:   Thu Aug 12 01:58:28 2010 +0000

     ctcm: rename READ/WRITE defines to avoid redefinitions


The analysis is as follows:

drivers/s390/net/ctcm_sysfs.c:43:8: note: Assuming that condition 'priv' 
is not redundant
  if (!(priv && priv->channel[CTCM_READ] && ndev)) {
        ^
drivers/s390/net/ctcm_sysfs.c:42:9: note: Null pointer dereference
  ndev = priv->channel[CTCM_READ]->netdev;

The code in question is as follows:

         ndev = priv->channel[CTCM_READ]->netdev;

         ^^ priv may be null, as per check below but it is being 
dereferenced when assigning ndev

         if (!(priv && priv->channel[CTCM_READ] && ndev)) {
                 CTCM_DBF_TEXT(SETUP, CTC_DBF_ERROR, "bfnondev");
                 return -ENODEV;
         }

Colin

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
From: Alaa Mohamed @ 2022-04-24 18:59 UTC (permalink / raw)
  To: Julia Lawall
  Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
	anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
	claudiu.manoil, alexandre.belloni, shshaikh, manishc, razor,
	intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
	bridge
In-Reply-To: <alpine.DEB.2.22.394.2204242054350.21756@hadrien>


On ٢٤‏/٤‏/٢٠٢٢ ٢٠:٥٦, Julia Lawall wrote:
>
> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>
>> On ٢٤/٤/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>>> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>>>
>>>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
>>> It could be helpful to say more about what adding extack support involves.
>>>
>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>> ---
>>>> changes in V2:
>>>> 	- fix spelling vxlan_fdb_delete
>>>> 	- add missing braces
>>>> 	- edit error message
>>>> ---
>>>> changes in V3:
>>>> 	fix errors reported by checkpatch.pl
>>> But your changes would seem to also be introducing more checkpatch errors,
>>> because the Linux kernel coding style puts a space before an { at the
>>> start of an if branch.
>> I ran checkpatch script before submitting this patch and got no error
> OK, whether checkpatch complains or not doesn't really matter.  The point
> is that in the Linux kernel, one writes:
>
> if (...) {
>
> and not
>
> if (...){
>
> You can see other examples of ifs in the Linux kernel.


Yes, got it. I will fix it.


Thanks,

Alaa

>
> julia
>
>>> julia
>>>
>>>> ---
>>>>    drivers/net/vxlan/vxlan_core.c | 36 +++++++++++++++++++++++-----------
>>>>    1 file changed, 25 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/net/vxlan/vxlan_core.c
>>>> b/drivers/net/vxlan/vxlan_core.c
>>>> index cf2f60037340..4e1886655101 100644
>>>> --- a/drivers/net/vxlan/vxlan_core.c
>>>> +++ b/drivers/net/vxlan/vxlan_core.c
>>>> @@ -1129,19 +1129,23 @@ static void vxlan_fdb_dst_destroy(struct vxlan_dev
>>>> *vxlan, struct vxlan_fdb *f,
>>>>
>>>>    static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
>>>>    			   union vxlan_addr *ip, __be16 *port, __be32
>>>> *src_vni,
>>>> -			   __be32 *vni, u32 *ifindex, u32 *nhid)
>>>> +			   __be32 *vni, u32 *ifindex, u32 *nhid, struct
>>>> netlink_ext_ack *extack)
>>>>    {
>>>>    	struct net *net = dev_net(vxlan->dev);
>>>>    	int err;
>>>>
>>>>    	if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
>>>> -	    tb[NDA_PORT]))
>>>> -		return -EINVAL;
>>>> +	    tb[NDA_PORT])){
>>>> +			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are
>>>> mutually exclusive with NH_ID");
>>>> +			return -EINVAL;
>>>> +		}
>>>>
>>>>    	if (tb[NDA_DST]) {
>>>>    		err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
>>>> -		if (err)
>>>> +		if (err){
>>>> +			NL_SET_ERR_MSG(extack, "Unsupported address family");
>>>>    			return err;
>>>> +		}
>>>>    	} else {
>>>>    		union vxlan_addr *remote = &vxlan->default_dst.remote_ip;
>>>>
>>>> @@ -1157,24 +1161,30 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
>>>> struct vxlan_dev *vxlan,
>>>>    	}
>>>>
>>>>    	if (tb[NDA_PORT]) {
>>>> -		if (nla_len(tb[NDA_PORT]) != sizeof(__be16))
>>>> +		if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
>>>> +			NL_SET_ERR_MSG(extack, "Invalid vxlan port");
>>>>    			return -EINVAL;
>>>> +		}
>>>>    		*port = nla_get_be16(tb[NDA_PORT]);
>>>>    	} else {
>>>>    		*port = vxlan->cfg.dst_port;
>>>>    	}
>>>>
>>>>    	if (tb[NDA_VNI]) {
>>>> -		if (nla_len(tb[NDA_VNI]) != sizeof(u32))
>>>> +		if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
>>>> +			NL_SET_ERR_MSG(extack, "Invalid vni");
>>>>    			return -EINVAL;
>>>> +		}
>>>>    		*vni = cpu_to_be32(nla_get_u32(tb[NDA_VNI]));
>>>>    	} else {
>>>>    		*vni = vxlan->default_dst.remote_vni;
>>>>    	}
>>>>
>>>>    	if (tb[NDA_SRC_VNI]) {
>>>> -		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32))
>>>> +		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
>>>> +			NL_SET_ERR_MSG(extack, "Invalid src vni");
>>>>    			return -EINVAL;
>>>> +		}
>>>>    		*src_vni = cpu_to_be32(nla_get_u32(tb[NDA_SRC_VNI]));
>>>>    	} else {
>>>>    		*src_vni = vxlan->default_dst.remote_vni;
>>>> @@ -1183,12 +1193,16 @@ static int vxlan_fdb_parse(struct nlattr *tb[],
>>>> struct vxlan_dev *vxlan,
>>>>    	if (tb[NDA_IFINDEX]) {
>>>>    		struct net_device *tdev;
>>>>
>>>> -		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
>>>> +		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
>>>> +			NL_SET_ERR_MSG(extack, "Invalid ifindex");
>>>>    			return -EINVAL;
>>>> +		}
>>>>    		*ifindex = nla_get_u32(tb[NDA_IFINDEX]);
>>>>    		tdev = __dev_get_by_index(net, *ifindex);
>>>> -		if (!tdev)
>>>> +		if (!tdev){
>>>> +			NL_SET_ERR_MSG(extack,"Device not found");
>>>>    			return -EADDRNOTAVAIL;
>>>> +		}
>>>>    	} else {
>>>>    		*ifindex = 0;
>>>>    	}
>>>> @@ -1226,7 +1240,7 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct
>>>> nlattr *tb[],
>>>>    		return -EINVAL;
>>>>
>>>>    	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>>>> -			      &nhid);
>>>> +			      &nhid, extack);
>>>>    	if (err)
>>>>    		return err;
>>>>
>>>> @@ -1291,7 +1305,7 @@ static int vxlan_fdb_delete(struct ndmsg *ndm,
>>>> struct nlattr *tb[],
>>>>    	int err;
>>>>
>>>>    	err = vxlan_fdb_parse(tb, vxlan, &ip, &port, &src_vni, &vni, &ifindex,
>>>> -			      &nhid);
>>>> +			      &nhid, extack);
>>>>    	if (err)
>>>>    		return err;
>>>>
>>>> --
>>>> 2.36.0
>>>>
>>>>
>>>>
> >

^ permalink raw reply

* Re: [PATCH net-next v3 1/2] rtnetlink: add extack support in fdb del handlers
From: Nikolay Aleksandrov @ 2022-04-24 19:02 UTC (permalink / raw)
  To: Alaa Mohamed, netdev
  Cc: outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
	anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
	claudiu.manoil, alexandre.belloni, shshaikh, manishc,
	intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
	bridge
In-Reply-To: <c3a882e4fb6f9228f704ebe3c1fcace14ee6cdf2.1650800975.git.eng.alaamohamedsoliman.am@gmail.com>

On 24/04/2022 15:09, Alaa Mohamed wrote:
> Add extack support to .ndo_fdb_del in netdevice.h and
> all related methods.
> 
> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
> ---
> changes in V3:
>         fix errors reported by checkpatch.pl
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c        | 4 ++--
>  drivers/net/ethernet/mscc/ocelot_net.c           | 4 ++--
>  drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +-
>  drivers/net/macvlan.c                            | 2 +-
>  drivers/net/vxlan/vxlan_core.c                   | 2 +-
>  include/linux/netdevice.h                        | 2 +-
>  net/bridge/br_fdb.c                              | 2 +-
>  net/bridge/br_private.h                          | 2 +-
>  net/core/rtnetlink.c                             | 4 ++--
>  9 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index d768925785ca..7b55d8d94803 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5678,10 +5678,10 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
>  static int
>  ice_fdb_del(struct ndmsg *ndm, __always_unused struct nlattr *tb[],
>  	    struct net_device *dev, const unsigned char *addr,
> -	    __always_unused u16 vid)
> +	    __always_unused u16 vid, struct netlink_ext_ack *extack)
>  {
>  	int err;
> -
> +

What's changed here?

>  	if (ndm->ndm_state & NUD_PERMANENT) {
>  		netdev_err(dev, "FDB only supports static addresses\n");
>  		return -EINVAL;
> diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
> index 247bc105bdd2..e07c64e3159c 100644
> --- a/drivers/net/ethernet/mscc/ocelot_net.c
> +++ b/drivers/net/ethernet/mscc/ocelot_net.c
> @@ -774,14 +774,14 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> 
>  static int ocelot_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>  			       struct net_device *dev,
> -			       const unsigned char *addr, u16 vid)
> +			       const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>  {
>  	struct ocelot_port_private *priv = netdev_priv(dev);
>  	struct ocelot_port *ocelot_port = &priv->port;
>  	struct ocelot *ocelot = ocelot_port->ocelot;
>  	int port = priv->chip_port;
> 
> -	return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge);
> +	return ocelot_fdb_del(ocelot, port, addr, vid, ocelot_port->bridge, extack);
>  }
> 
>  static int ocelot_port_fdb_dump(struct sk_buff *skb,
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> index d320567b2cca..51fa23418f6a 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
> @@ -368,7 +368,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)
> 
>  static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>  			struct net_device *netdev,
> -			const unsigned char *addr, u16 vid)
> +			const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>  {
>  	struct qlcnic_adapter *adapter = netdev_priv(netdev);
>  	int err = -EOPNOTSUPP;
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 069e8824c264..ffd34d9f7049 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -1017,7 +1017,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
> 
>  static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
>  			   struct net_device *dev,
> -			   const unsigned char *addr, u16 vid)
> +			   const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>  {
>  	struct macvlan_dev *vlan = netdev_priv(dev);
>  	int err = -EINVAL;
> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index de97ff98d36e..cf2f60037340 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -1280,7 +1280,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,
>  /* Delete entry (via netlink) */
>  static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>  			    struct net_device *dev,
> -			    const unsigned char *addr, u16 vid)
> +			    const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>  {
>  	struct vxlan_dev *vxlan = netdev_priv(dev);
>  	union vxlan_addr ip;
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 28ea4f8269d4..d0d2a8f33c73 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1509,7 +1509,7 @@ struct net_device_ops {
>  					       struct nlattr *tb[],
>  					       struct net_device *dev,
>  					       const unsigned char *addr,
> -					       u16 vid);
> +					       u16 vid, struct netlink_ext_ack *extack);
>  	int			(*ndo_fdb_dump)(struct sk_buff *skb,
>  						struct netlink_callback *cb,
>  						struct net_device *dev,
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 6ccda68bd473..5bfce2e9a553 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -1110,7 +1110,7 @@ static int __br_fdb_delete(struct net_bridge *br,
>  /* Remove neighbor entry with RTM_DELNEIGH */
>  int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
>  		  struct net_device *dev,
> -		  const unsigned char *addr, u16 vid)
> +		  const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack)
>  {
>  	struct net_bridge_vlan_group *vg;
>  	struct net_bridge_port *p = NULL;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 18ccc3d5d296..95348c1c9ce5 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -780,7 +780,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>  		   const unsigned char *addr, u16 vid, unsigned long flags);
> 
>  int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[],
> -		  struct net_device *dev, const unsigned char *addr, u16 vid);
> +		  struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);

This is way too long (111 chars) and checkpatch should've complained about it.
WARNING: line length of 111 exceeds 100 columns
#234: FILE: net/bridge/br_private.h:782:
+		  struct net_device *dev, const unsigned char *addr, u16 vid, struct netlink_ext_ack *extack);

>  int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
>  	       const unsigned char *addr, u16 vid, u16 nlh_flags,
>  	       struct netlink_ext_ack *extack);
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 4041b3e2e8ec..99b30ae58a47 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4223,7 +4223,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>  		const struct net_device_ops *ops = br_dev->netdev_ops;
> 
>  		if (ops->ndo_fdb_del)
> -			err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid);
> +			err = ops->ndo_fdb_del(ndm, tb, dev, addr, vid, extack);
> 
>  		if (err)
>  			goto out;
> @@ -4235,7 +4235,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (ndm->ndm_flags & NTF_SELF) {
>  		if (dev->netdev_ops->ndo_fdb_del)
>  			err = dev->netdev_ops->ndo_fdb_del(ndm, tb, dev, addr,
> -							   vid);
> +							   vid, extack);
>  		else
>  			err = ndo_dflt_fdb_del(ndm, tb, dev, addr, vid);
> 
> --
> 2.36.0
> 


^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
From: Nikolay Aleksandrov @ 2022-04-24 19:03 UTC (permalink / raw)
  To: Alaa Mohamed, Julia Lawall
  Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
	anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
	claudiu.manoil, alexandre.belloni, shshaikh, manishc,
	intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
	bridge
In-Reply-To: <06622e4c-b9a5-1c4f-2764-a72733000b4e@gmail.com>

On 24/04/2022 21:52, Alaa Mohamed wrote:
> 
> On ٢٤‏/٤‏/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>>
>> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>>
>>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
>> It could be helpful to say more about what adding extack support involves.
>>
>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>> ---
>>> changes in V2:
>>>     - fix spelling vxlan_fdb_delete
>>>     - add missing braces
>>>     - edit error message
>>> ---
>>> changes in V3:
>>>     fix errors reported by checkpatch.pl
>> But your changes would seem to also be introducing more checkpatch errors,
>> because the Linux kernel coding style puts a space before an { at the
>> start of an if branch.
> I ran checkpatch script before submitting this patch and got no error

This is what I got:
WARNING: suspect code indent for conditional statements (8, 24)
#366: FILE: drivers/net/vxlan/vxlan_core.c:1137:
 	if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
[...]
+			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");

WARNING: line length of 111 exceeds 100 columns
#370: FILE: drivers/net/vxlan/vxlan_core.c:1139:
+			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");

ERROR: space required before the open brace '{'
#377: FILE: drivers/net/vxlan/vxlan_core.c:1145:
+		if (err){

ERROR: space required before the open brace '{'
#389: FILE: drivers/net/vxlan/vxlan_core.c:1164:
+		if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){

ERROR: space required before the open brace '{'
#400: FILE: drivers/net/vxlan/vxlan_core.c:1174:
+		if (nla_len(tb[NDA_VNI]) != sizeof(u32)){

ERROR: space required before the open brace '{'
#411: FILE: drivers/net/vxlan/vxlan_core.c:1184:
+		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){

ERROR: space required before the open brace '{'
#423: FILE: drivers/net/vxlan/vxlan_core.c:1196:
+		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){

ERROR: space required before the open brace '{'
#430: FILE: drivers/net/vxlan/vxlan_core.c:1202:
+		if (!tdev){

ERROR: space required after that ',' (ctx:VxV)
#431: FILE: drivers/net/vxlan/vxlan_core.c:1203:
+			NL_SET_ERR_MSG(extack,"Device not found");




^ permalink raw reply

* Re: [PATCH v2] brcmfmac: of: introduce new property to allow disable PNO
From: Arend van Spriel @ 2022-04-24 19:13 UTC (permalink / raw)
  To: Kalle Valo, Hermes Zhang
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, David S. Miller,
	Jakub Kicinski, Paolo Abeni, kernel, Hermes Zhang, linux-wireless,
	brcm80211-dev-list.pdl, SHA-cyfmac-dev-list, netdev, linux-kernel
In-Reply-To: <8735i5odyo.fsf@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]

On 4/22/2022 7:59 AM, Kalle Valo wrote:
> Hermes Zhang <chenhui.zhang@axis.com> writes:
> 
>> From: Hermes Zhang <chenhuiz@axis.com>
>>
>> The PNO feature need to be disable for some scenario in different
>> product. This commit introduce a new property to allow the
>> product-specific toggling of this feature.
> 
> "some scenario"? That's not really helpful.

The firmware feature PNO is used to provide the scheduled scan 
functionality. User-space can choose whether or not to use scheduled 
scan. If the scheduled scan is not working I would rather see a bug 
report so it can be properly investigated.

Regards,
Arend

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

^ permalink raw reply

* Re: [PATCH] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Andrew Lunn @ 2022-04-24 19:18 UTC (permalink / raw)
  To: Marek Behún
  Cc: Nathan Rossi, netdev, linux-kernel, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller,
	Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220424165228.4030aea6@thinkpad>

On Sun, Apr 24, 2022 at 04:52:28PM +0200, Marek Behún wrote:
> On Sun, 24 Apr 2022 14:17:59 +0000
> Nathan Rossi <nathan@nathanrossi.com> wrote:
> 
> > The other port_hidden functions rely on the port_read/port_write
> > functions to access the hidden control port. These functions apply the
> > offset for port_base_addr where applicable. Update port_hidden_wait to
> > use the port_wait_bit so that port_base_addr offsets are accounted for
> > when waiting for the busy bit to change.
> > 
> > Without the offset the port_hidden_wait function would timeout on
> > devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> > devices that have a zero port_base_addr would operate correctly (e.g.
> > MV88E6390).
> 
> So basically the code is accessing the wrong register for devices with
> non-zero port_base_addr. This means that the patch should have a Fixes
> tag with the commit that introduced this bug, so that it gets
> backported to relevant stable versions.
> 
> Could you resend with Fixes tag?

I think that would be:

Fixes: 609070133aff ("net: dsa: mv88e6xxx: update code operating on hidden registers")

That patch moved the code out of chip.c into port_hidden. At the same
time, the functions got renamed from mv88e6390_hidden_read() to
mv886xxx_hidden_read(). 6390 does have zero-based ports, so was
correct before the rename. But the more generic name suggest it should
take into account devices which do not have zero based ports.

With the fixes tag added and based on met:

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

    Andrew

^ permalink raw reply

* Re: [PATCH net-next v3 2/2] net: vxlan: vxlan_core.c: Add extack support to vxlan_fdb_delete
From: Alaa Mohamed @ 2022-04-24 19:20 UTC (permalink / raw)
  To: Nikolay Aleksandrov, Julia Lawall
  Cc: netdev, outreachy, roopa, jdenham, sbrivio, jesse.brandeburg,
	anthony.l.nguyen, davem, kuba, pabeni, vladimir.oltean,
	claudiu.manoil, alexandre.belloni, shshaikh, manishc,
	intel-wired-lan, linux-kernel, UNGLinuxDriver, GR-Linux-NIC-Dev,
	bridge
In-Reply-To: <3b8c790d-9e90-d920-9580-8e736435f7f3@blackwall.org>


On ٢٤‏/٤‏/٢٠٢٢ ٢١:٠٣, Nikolay Aleksandrov wrote:
> On 24/04/2022 21:52, Alaa Mohamed wrote:
>> On ٢٤‏/٤‏/٢٠٢٢ ١٨:١٥, Julia Lawall wrote:
>>> On Sun, 24 Apr 2022, Alaa Mohamed wrote:
>>>
>>>> Add extack to vxlan_fdb_delete and vxlan_fdb_parse
>>> It could be helpful to say more about what adding extack support involves.
>>>
>>>> Signed-off-by: Alaa Mohamed <eng.alaamohamedsoliman.am@gmail.com>
>>>> ---
>>>> changes in V2:
>>>>      - fix spelling vxlan_fdb_delete
>>>>      - add missing braces
>>>>      - edit error message
>>>> ---
>>>> changes in V3:
>>>>      fix errors reported by checkpatch.pl
>>> But your changes would seem to also be introducing more checkpatch errors,
>>> because the Linux kernel coding style puts a space before an { at the
>>> start of an if branch.
>> I ran checkpatch script before submitting this patch and got no error
> This is what I got:
> WARNING: suspect code indent for conditional statements (8, 24)
> #366: FILE: drivers/net/vxlan/vxlan_core.c:1137:
>   	if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
> [...]
> +			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
>
> WARNING: line length of 111 exceeds 100 columns
> #370: FILE: drivers/net/vxlan/vxlan_core.c:1139:
> +			NL_SET_ERR_MSG(extack, "DST, VNI, ifindex and port are mutually exclusive with NH_ID");
>
> ERROR: space required before the open brace '{'
> #377: FILE: drivers/net/vxlan/vxlan_core.c:1145:
> +		if (err){
>
> ERROR: space required before the open brace '{'
> #389: FILE: drivers/net/vxlan/vxlan_core.c:1164:
> +		if (nla_len(tb[NDA_PORT]) != sizeof(__be16)){
>
> ERROR: space required before the open brace '{'
> #400: FILE: drivers/net/vxlan/vxlan_core.c:1174:
> +		if (nla_len(tb[NDA_VNI]) != sizeof(u32)){
>
> ERROR: space required before the open brace '{'
> #411: FILE: drivers/net/vxlan/vxlan_core.c:1184:
> +		if (nla_len(tb[NDA_SRC_VNI]) != sizeof(u32)){
>
> ERROR: space required before the open brace '{'
> #423: FILE: drivers/net/vxlan/vxlan_core.c:1196:
> +		if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32)){
>
> ERROR: space required before the open brace '{'
> #430: FILE: drivers/net/vxlan/vxlan_core.c:1202:
> +		if (!tdev){
>
> ERROR: space required after that ',' (ctx:VxV)
> #431: FILE: drivers/net/vxlan/vxlan_core.c:1203:
> +			NL_SET_ERR_MSG(extack,"Device not found");
>
>
Thank you for sending that , but I don't know why I missed that when I 
ran the script. Anyway, I fixed all these errors as Julia said.

Thanks again,

Alaa


^ permalink raw reply

* Re: [PATCH v2] net: dsa: mv88e6xxx: Fix port_hidden_wait to account for port_base_addr
From: Andrew Lunn @ 2022-04-24 19:26 UTC (permalink / raw)
  To: Nathan Rossi
  Cc: netdev, linux-kernel, Marek Behun, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, David S. Miller,
	Jakub Kicinski, Paolo Abeni
In-Reply-To: <20220424153143.323338-1-nathan@nathanrossi.com>

On Sun, Apr 24, 2022 at 03:31:43PM +0000, Nathan Rossi wrote:
> The other port_hidden functions rely on the port_read/port_write
> functions to access the hidden control port. These functions apply the
> offset for port_base_addr where applicable. Update port_hidden_wait to
> use the port_wait_bit so that port_base_addr offsets are accounted for
> when waiting for the busy bit to change.
> 
> Without the offset the port_hidden_wait function would timeout on
> devices that have a non-zero port_base_addr (e.g. MV88E6141), however
> devices that have a zero port_base_addr would operate correctly (e.g.
> MV88E6390).
> 
> Fixes: ea89098ef9a5 ("net: dsa: mv88x6xxx: mv88e6390 errata")

That is further back than needed. And due to the code moving around
and getting renamed, you are added extra burden on those doing the
back port for no actual gain.

Please verify what i suggested, 609070133aff1 is better and then
repost.

	Thanks
		Andrew

^ permalink raw reply


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