[PATCH] kni: fix device address set

[PATCH] kni: fix device address set

[Min Hu (Connor)]

Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
dmesg buffer get calltrace, info like:
[ 5965.847401] rte_kni: Creating kni...
[ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
[ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
[ 6225.653010] ------------[ cut here ]------------
[ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
[ 6225.832647] Call trace:
[ 6225.835083]  dev_addr_check+0xa0/0x144
[ 6225.838816]  dev_addr_flush+0x30/0x9c
[ 6225.842462]  free_netdev+0x8c/0x1e0
[ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
[ 6225.850281]  __fput+0x78/0x220
[ 6225.853327]  ____fput+0x1c/0x30
[ 6225.856455]  task_work_run+0x88/0xc0
[ 6225.860017]  do_exit+0x2fc/0x940
[ 6225.863232]  do_group_exit+0x40/0xac
[ 6225.866791]  get_signal+0x190/0x960
[ 6225.870265]  do_notify_resume+0x26c/0x1360
[ 6225.874346]  el0_interrupt+0x60/0xe0
[ 6225.877910]  __el0_irq_handler_common+0x18/0x24
[ 6225.882420]  el0t_64_irq_handler+0x14/0x20
[ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
[ 6225.890059] ---[ end trace 0000000000000000 ]---
[ 6245.598157] rte_kni: Creating kni...

The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
'dev_addr_set' to ensure that netdev->dev_addr should only be modified
via helpers('dev_addr_set'). 'dev_addr_check' will check if
netdev->dev_addr is modified by other ways, like 'memcpy'.

More info could get by referring to kernel patch:
https://patchwork.kernel.org/project/netdevbpf/patch/
20211118041501.3102861-8-kuba@kernel.org/
https://www.spinics.net/lists/netdev/msg764992.html

Fixes: ea6b39b5b847 ("kni: remove ethtool support")

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 kernel/linux/kni/kni_misc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 780187d8bf..046de0311f 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	struct kni_net *knet = net_generic(net, kni_net_id);
 	int ret;
 	struct rte_kni_device_info dev_info;
+	unsigned char mac_addr[ETH_ALEN];
 	struct net_device *net_dev = NULL;
 	struct kni_dev *kni, *dev, *n;
 
@@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 
 	/* if user has provided a valid mac address */
 	if (is_valid_ether_addr(dev_info.mac_addr))
-		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
+		memcpy(mac_addr, dev_info.mac_addr, ETH_ALEN);
 	else
 		/* Generate random MAC address. */
-		eth_random_addr(net_dev->dev_addr);
+		eth_random_addr(mac_addr);
+#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
+	memcpy(net_dev->dev_addr, mac_addr, ETH_ALEN);
+#else
+	dev_addr_set(net_dev, mac_addr);
+#endif
 
 	if (dev_info.mtu)
 		net_dev->mtu = dev_info.mtu;
-- 
2.33.0

Re: [PATCH] kni: fix device address set

[Stephen Hemminger]

On Wed, 6 Apr 2022 16:22:13 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

>  	/* if user has provided a valid mac address */
>  	if (is_valid_ether_addr(dev_info.mac_addr))
> -		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
> +		memcpy(mac_addr, dev_info.mac_addr, ETH_ALEN);

Minor enhancement. Could this use ether_addr_copy instead?
		ether_addr_copy(mac_addr, dev_info.mac_addr);

Re: [PATCH] kni: fix device address set

[Min Hu (Connor)]

Hi, Stephen,
  I think this is a good option, but the macro definition is like:
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
+#define ether_addr_copy(dst, src) memcpy(dst, src, ETH_ALEN)
+#endif

@Ferry, why is it limited for "LINUX_VERSION_CODE < KERNEL_VERSION(3, 
14, 0)" ?


在 2022/4/6 23:17, Stephen Hemminger 写道:
> On Wed, 6 Apr 2022 16:22:13 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
>>   	/* if user has provided a valid mac address */
>>   	if (is_valid_ether_addr(dev_info.mac_addr))
>> -		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
>> +		memcpy(mac_addr, dev_info.mac_addr, ETH_ALEN);
> 
> Minor enhancement. Could this use ether_addr_copy instead?
> 		ether_addr_copy(mac_addr, dev_info.mac_addr);
> .
> 

Re: [PATCH] kni: fix device address set

[Stephen Hemminger]

On Thu, 7 Apr 2022 08:44:23 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> Hi, Stephen,
>   I think this is a good option, but the macro definition is like:
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
> +#define ether_addr_copy(dst, src) memcpy(dst, src, ETH_ALEN)
> +#endif

Minimal supported DPDK version is 4.4 now. So this is not a problem.

Note: 4.4 kernel reached end of support window in Febrary 2022.
It is supported by the SLTS project but it would be unwise
to use later DPDK on a kernel that is stuck being supported until 2036.

Apparently my patch to update it to current LTS 4.9 is sitting
stuck in patchwork.

Re: [PATCH] kni: fix device address set

[Min Hu (Connor)]

Hi,

在 2022/4/7 11:18, Stephen Hemminger 写道:
> On Thu, 7 Apr 2022 08:44:23 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
>> Hi, Stephen,
>>    I think this is a good option, but the macro definition is like:
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
>> +#define ether_addr_copy(dst, src) memcpy(dst, src, ETH_ALEN)
>> +#endif
> 
> Minimal supported DPDK version is 4.4 now. So this is not a problem.
I did not catch your meaning. I think this is a problem.
Acording to current definition, we can only use "ether_addr_copy" on 
linux platform of version 3.13.0(or below).

How about the situaction on other linux version, like 3.14.0(or upper)?


> 
> Note: 4.4 kernel reached end of support window in Febrary 2022.
> It is supported by the SLTS project but it would be unwise
> to use later DPDK on a kernel that is stuck being supported until 2036.
> 
> Apparently my patch to update it to current LTS 4.9 is sitting
> stuck in patchwork.
> .
> 

Re: [PATCH] kni: fix device address set

[Thomas Monjalon]

07/04/2022 02:44, Min Hu (Connor):
> Hi, Stephen,
>   I think this is a good option, but the macro definition is like:
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
> +#define ether_addr_copy(dst, src) memcpy(dst, src, ETH_ALEN)
> +#endif
> 
> @Ferry, why is it limited for "LINUX_VERSION_CODE < KERNEL_VERSION(3, 
> 14, 0)" ?

I guess that's because it is defined in "new kernels" so we need
a definition in DPDK for old kernels.

Re: [PATCH] kni: fix device address set

[Min Hu (Connor)]

OK, I see, thanks Thomas.

在 2022/4/7 15:42, Thomas Monjalon 写道:
> 07/04/2022 02:44, Min Hu (Connor):
>> Hi, Stephen,
>>    I think this is a good option, but the macro definition is like:
>> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
>> +#define ether_addr_copy(dst, src) memcpy(dst, src, ETH_ALEN)
>> +#endif
>>
>> @Ferry, why is it limited for "LINUX_VERSION_CODE < KERNEL_VERSION(3,
>> 14, 0)" ?
> 
> I guess that's because it is defined in "new kernels" so we need
> a definition in DPDK for old kernels.
> 
> 
> 
> .
> 

[PATCH v2] kni: fix device address set

[Min Hu (Connor)]

Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
dmesg buffer get calltrace, info like:
[ 5965.847401] rte_kni: Creating kni...
[ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
[ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
[ 6225.653010] ------------[ cut here ]------------
[ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
[ 6225.832647] Call trace:
[ 6225.835083]  dev_addr_check+0xa0/0x144
[ 6225.838816]  dev_addr_flush+0x30/0x9c
[ 6225.842462]  free_netdev+0x8c/0x1e0
[ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
[ 6225.850281]  __fput+0x78/0x220
[ 6225.853327]  ____fput+0x1c/0x30
[ 6225.856455]  task_work_run+0x88/0xc0
[ 6225.860017]  do_exit+0x2fc/0x940
[ 6225.863232]  do_group_exit+0x40/0xac
[ 6225.866791]  get_signal+0x190/0x960
[ 6225.870265]  do_notify_resume+0x26c/0x1360
[ 6225.874346]  el0_interrupt+0x60/0xe0
[ 6225.877910]  __el0_irq_handler_common+0x18/0x24
[ 6225.882420]  el0t_64_irq_handler+0x14/0x20
[ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
[ 6225.890059] ---[ end trace 0000000000000000 ]---
[ 6245.598157] rte_kni: Creating kni...

The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
'dev_addr_set' to ensure that netdev->dev_addr should only be modified
via helpers('dev_addr_set'). 'dev_addr_check' will check if
netdev->dev_addr is modified by other ways, like 'memcpy'.

More info could get by referring to kernel patch:
https://patchwork.kernel.org/project/netdevbpf/patch/
20211118041501.3102861-8-kuba@kernel.org/
https://www.spinics.net/lists/netdev/msg764992.html

Fixes: ea6b39b5b847 ("kni: remove ethtool support")

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v2:
* change 'memcpy' to 'ether_addr_copy' to copy device addr.
---
 kernel/linux/kni/kni_misc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 780187d8bf..17b6c21a36 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	struct kni_net *knet = net_generic(net, kni_net_id);
 	int ret;
 	struct rte_kni_device_info dev_info;
+	unsigned char mac_addr[ETH_ALEN];
 	struct net_device *net_dev = NULL;
 	struct kni_dev *kni, *dev, *n;
 
@@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 
 	/* if user has provided a valid mac address */
 	if (is_valid_ether_addr(dev_info.mac_addr))
-		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
+		ether_addr_copy(mac_addr, dev_info.mac_addr)
 	else
 		/* Generate random MAC address. */
-		eth_random_addr(net_dev->dev_addr);
+		eth_random_addr(mac_addr);
+#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
+	ether_addr_copy(net_dev->dev_addr, mac_addr)
+#else
+	dev_addr_set(net_dev, mac_addr);
+#endif
 
 	if (dev_info.mtu)
 		net_dev->mtu = dev_info.mtu;
-- 
2.33.0

Re: [PATCH v2] kni: fix device address set

[Min Hu (Connor)]

Hi, all,
	any one has any comments for this patch?

在 2022/4/7 16:25, Min Hu (Connor) 写道:
> Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
> dmesg buffer get calltrace, info like:
> [ 5965.847401] rte_kni: Creating kni...
> [ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
> [ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
> [ 6225.653010] ------------[ cut here ]------------
> [ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
> [ 6225.832647] Call trace:
> [ 6225.835083]  dev_addr_check+0xa0/0x144
> [ 6225.838816]  dev_addr_flush+0x30/0x9c
> [ 6225.842462]  free_netdev+0x8c/0x1e0
> [ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
> [ 6225.850281]  __fput+0x78/0x220
> [ 6225.853327]  ____fput+0x1c/0x30
> [ 6225.856455]  task_work_run+0x88/0xc0
> [ 6225.860017]  do_exit+0x2fc/0x940
> [ 6225.863232]  do_group_exit+0x40/0xac
> [ 6225.866791]  get_signal+0x190/0x960
> [ 6225.870265]  do_notify_resume+0x26c/0x1360
> [ 6225.874346]  el0_interrupt+0x60/0xe0
> [ 6225.877910]  __el0_irq_handler_common+0x18/0x24
> [ 6225.882420]  el0t_64_irq_handler+0x14/0x20
> [ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
> [ 6225.890059] ---[ end trace 0000000000000000 ]---
> [ 6245.598157] rte_kni: Creating kni...
> 
> The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
> 'dev_addr_set' to ensure that netdev->dev_addr should only be modified
> via helpers('dev_addr_set'). 'dev_addr_check' will check if
> netdev->dev_addr is modified by other ways, like 'memcpy'.
> 
> More info could get by referring to kernel patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/
> 20211118041501.3102861-8-kuba@kernel.org/
> https://www.spinics.net/lists/netdev/msg764992.html
> 
> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v2:
> * change 'memcpy' to 'ether_addr_copy' to copy device addr.
> ---
>   kernel/linux/kni/kni_misc.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index 780187d8bf..17b6c21a36 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   	struct kni_net *knet = net_generic(net, kni_net_id);
>   	int ret;
>   	struct rte_kni_device_info dev_info;
> +	unsigned char mac_addr[ETH_ALEN];
>   	struct net_device *net_dev = NULL;
>   	struct kni_dev *kni, *dev, *n;
>   
> @@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   
>   	/* if user has provided a valid mac address */
>   	if (is_valid_ether_addr(dev_info.mac_addr))
> -		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
> +		ether_addr_copy(mac_addr, dev_info.mac_addr)
>   	else
>   		/* Generate random MAC address. */
> -		eth_random_addr(net_dev->dev_addr);
> +		eth_random_addr(mac_addr);
> +#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
> +	ether_addr_copy(net_dev->dev_addr, mac_addr)
> +#else
> +	dev_addr_set(net_dev, mac_addr);
> +#endif
>   
>   	if (dev_info.mtu)
>   		net_dev->mtu = dev_info.mtu;
> 

[PATCH v3] kni: fix device address set

[Min Hu (Connor)]

Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
dmesg buffer get calltrace, info like:
[ 5965.847401] rte_kni: Creating kni...
[ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
[ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
[ 6225.653010] ------------[ cut here ]------------
[ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
[ 6225.832647] Call trace:
[ 6225.835083]  dev_addr_check+0xa0/0x144
[ 6225.838816]  dev_addr_flush+0x30/0x9c
[ 6225.842462]  free_netdev+0x8c/0x1e0
[ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
[ 6225.850281]  __fput+0x78/0x220
[ 6225.853327]  ____fput+0x1c/0x30
[ 6225.856455]  task_work_run+0x88/0xc0
[ 6225.860017]  do_exit+0x2fc/0x940
[ 6225.863232]  do_group_exit+0x40/0xac
[ 6225.866791]  get_signal+0x190/0x960
[ 6225.870265]  do_notify_resume+0x26c/0x1360
[ 6225.874346]  el0_interrupt+0x60/0xe0
[ 6225.877910]  __el0_irq_handler_common+0x18/0x24
[ 6225.882420]  el0t_64_irq_handler+0x14/0x20
[ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
[ 6225.890059] ---[ end trace 0000000000000000 ]---
[ 6245.598157] rte_kni: Creating kni...

The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
'dev_addr_set' to ensure that netdev->dev_addr should only be modified
via helpers('dev_addr_set'). 'dev_addr_check' will check if
netdev->dev_addr is modified by other ways, like 'memcpy'.

More info could get by referring to kernel patch:
https://patchwork.kernel.org/project/netdevbpf/patch/
20211118041501.3102861-8-kuba@kernel.org/
https://www.spinics.net/lists/netdev/msg764992.html

Fixes: ea6b39b5b847 ("kni: remove ethtool support")

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v3:
* fix compiling errors.

v2:
* change 'memcpy' to 'ether_addr_copy' to copy device addr.
---
 kernel/linux/kni/kni_misc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 780187d8bf..9c5d8c5576 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	struct kni_net *knet = net_generic(net, kni_net_id);
 	int ret;
 	struct rte_kni_device_info dev_info;
+	unsigned char mac_addr[ETH_ALEN];
 	struct net_device *net_dev = NULL;
 	struct kni_dev *kni, *dev, *n;
 
@@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 
 	/* if user has provided a valid mac address */
 	if (is_valid_ether_addr(dev_info.mac_addr))
-		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
+		ether_addr_copy(mac_addr, dev_info.mac_addr);
 	else
 		/* Generate random MAC address. */
-		eth_random_addr(net_dev->dev_addr);
+		eth_random_addr(mac_addr);
+#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
+	ether_addr_copy(net_dev->dev_addr, mac_addr)
+#else
+	dev_addr_set(net_dev, mac_addr);
+#endif
 
 	if (dev_info.mtu)
 		net_dev->mtu = dev_info.mtu;
-- 
2.33.0

Re: [PATCH v3] kni: fix device address set

[Andrew Rybchenko]

On 5/23/22 12:24, Min Hu (Connor) wrote:
> Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
> dmesg buffer get calltrace, info like:
> [ 5965.847401] rte_kni: Creating kni...
> [ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
> [ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
> [ 6225.653010] ------------[ cut here ]------------
> [ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
> [ 6225.832647] Call trace:
> [ 6225.835083]  dev_addr_check+0xa0/0x144
> [ 6225.838816]  dev_addr_flush+0x30/0x9c
> [ 6225.842462]  free_netdev+0x8c/0x1e0
> [ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
> [ 6225.850281]  __fput+0x78/0x220
> [ 6225.853327]  ____fput+0x1c/0x30
> [ 6225.856455]  task_work_run+0x88/0xc0
> [ 6225.860017]  do_exit+0x2fc/0x940
> [ 6225.863232]  do_group_exit+0x40/0xac
> [ 6225.866791]  get_signal+0x190/0x960
> [ 6225.870265]  do_notify_resume+0x26c/0x1360
> [ 6225.874346]  el0_interrupt+0x60/0xe0
> [ 6225.877910]  __el0_irq_handler_common+0x18/0x24
> [ 6225.882420]  el0t_64_irq_handler+0x14/0x20
> [ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
> [ 6225.890059] ---[ end trace 0000000000000000 ]---
> [ 6245.598157] rte_kni: Creating kni...
> 
> The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
> 'dev_addr_set' to ensure that netdev->dev_addr should only be modified
> via helpers('dev_addr_set'). 'dev_addr_check' will check if
> netdev->dev_addr is modified by other ways, like 'memcpy'.
> 
> More info could get by referring to kernel patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/
> 20211118041501.3102861-8-kuba@kernel.org/
> https://www.spinics.net/lists/netdev/msg764992.html
> 
> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v3:
> * fix compiling errors.
> 
> v2:
> * change 'memcpy' to 'ether_addr_copy' to copy device addr.
> ---
>   kernel/linux/kni/kni_misc.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index 780187d8bf..9c5d8c5576 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   	struct kni_net *knet = net_generic(net, kni_net_id);
>   	int ret;
>   	struct rte_kni_device_info dev_info;
> +	unsigned char mac_addr[ETH_ALEN];
>   	struct net_device *net_dev = NULL;
>   	struct kni_dev *kni, *dev, *n;
>   
> @@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   
>   	/* if user has provided a valid mac address */
>   	if (is_valid_ether_addr(dev_info.mac_addr))
> -		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
> +		ether_addr_copy(mac_addr, dev_info.mac_addr);
>   	else
>   		/* Generate random MAC address. */
> -		eth_random_addr(net_dev->dev_addr);
> +		eth_random_addr(mac_addr);
> +#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
> +	ether_addr_copy(net_dev->dev_addr, mac_addr)

Since semicolon is missing above, it breaks build.
Please, fix and send v4.

> +#else
> +	dev_addr_set(net_dev, mac_addr);
> +#endif
>   
>   	if (dev_info.mtu)
>   		net_dev->mtu = dev_info.mtu;

[PATCH v4] kni: fix device address set

[Min Hu (Connor)]

Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
dmesg buffer get calltrace, info like:
[ 5965.847401] rte_kni: Creating kni...
[ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
[ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
[ 6225.653010] ------------[ cut here ]------------
[ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
[ 6225.832647] Call trace:
[ 6225.835083]  dev_addr_check+0xa0/0x144
[ 6225.838816]  dev_addr_flush+0x30/0x9c
[ 6225.842462]  free_netdev+0x8c/0x1e0
[ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
[ 6225.850281]  __fput+0x78/0x220
[ 6225.853327]  ____fput+0x1c/0x30
[ 6225.856455]  task_work_run+0x88/0xc0
[ 6225.860017]  do_exit+0x2fc/0x940
[ 6225.863232]  do_group_exit+0x40/0xac
[ 6225.866791]  get_signal+0x190/0x960
[ 6225.870265]  do_notify_resume+0x26c/0x1360
[ 6225.874346]  el0_interrupt+0x60/0xe0
[ 6225.877910]  __el0_irq_handler_common+0x18/0x24
[ 6225.882420]  el0t_64_irq_handler+0x14/0x20
[ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
[ 6225.890059] ---[ end trace 0000000000000000 ]---
[ 6245.598157] rte_kni: Creating kni...

The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
'dev_addr_set' to ensure that netdev->dev_addr should only be modified
via helpers('dev_addr_set'). 'dev_addr_check' will check if
netdev->dev_addr is modified by other ways, like 'memcpy'.

More info could get by referring to kernel patch:
https://patchwork.kernel.org/project/netdevbpf/patch/
20211118041501.3102861-8-kuba@kernel.org/
https://www.spinics.net/lists/netdev/msg764992.html

Fixes: ea6b39b5b847 ("kni: remove ethtool support")

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v3,v4:
* fix compiling errors.

v2:
* change 'memcpy' to 'ether_addr_copy' to copy device addr.
---
 kernel/linux/kni/kni_misc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 780187d8bf..9c5d8c5576 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	struct kni_net *knet = net_generic(net, kni_net_id);
 	int ret;
 	struct rte_kni_device_info dev_info;
+	unsigned char mac_addr[ETH_ALEN];
 	struct net_device *net_dev = NULL;
 	struct kni_dev *kni, *dev, *n;
 
@@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 
 	/* if user has provided a valid mac address */
 	if (is_valid_ether_addr(dev_info.mac_addr))
-		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
+		ether_addr_copy(mac_addr, dev_info.mac_addr);
 	else
 		/* Generate random MAC address. */
-		eth_random_addr(net_dev->dev_addr);
+		eth_random_addr(mac_addr);
+#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
+	ether_addr_copy(net_dev->dev_addr, mac_addr);
+#else
+	dev_addr_set(net_dev, mac_addr);
+#endif
 
 	if (dev_info.mtu)
 		net_dev->mtu = dev_info.mtu;
-- 
2.33.0

Re: [PATCH v3] kni: fix device address set

[Min Hu (Connor)]

Hi, Andrew,
	v4 has been sent, thanks.

在 2022/5/31 23:32, Andrew Rybchenko 写道:
> On 5/23/22 12:24, Min Hu (Connor) wrote:
>> Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
>> dmesg buffer get calltrace, info like:
>> [ 5965.847401] rte_kni: Creating kni...
>> [ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 
>> 00..
>> [ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 
>> 00..
>> [ 6225.653010] ------------[ cut here ]------------
>> [ 6225.657609] netdevice: vEth0 (unregistered): Incorrect 
>> netdev->dev_addr
>> [ 6225.832647] Call trace:
>> [ 6225.835083]  dev_addr_check+0xa0/0x144
>> [ 6225.838816]  dev_addr_flush+0x30/0x9c
>> [ 6225.842462]  free_netdev+0x8c/0x1e0
>> [ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
>> [ 6225.850281]  __fput+0x78/0x220
>> [ 6225.853327]  ____fput+0x1c/0x30
>> [ 6225.856455]  task_work_run+0x88/0xc0
>> [ 6225.860017]  do_exit+0x2fc/0x940
>> [ 6225.863232]  do_group_exit+0x40/0xac
>> [ 6225.866791]  get_signal+0x190/0x960
>> [ 6225.870265]  do_notify_resume+0x26c/0x1360
>> [ 6225.874346]  el0_interrupt+0x60/0xe0
>> [ 6225.877910]  __el0_irq_handler_common+0x18/0x24
>> [ 6225.882420]  el0t_64_irq_handler+0x14/0x20
>> [ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
>> [ 6225.890059] ---[ end trace 0000000000000000 ]---
>> [ 6245.598157] rte_kni: Creating kni...
>>
>> The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
>> 'dev_addr_set' to ensure that netdev->dev_addr should only be modified
>> via helpers('dev_addr_set'). 'dev_addr_check' will check if
>> netdev->dev_addr is modified by other ways, like 'memcpy'.
>>
>> More info could get by referring to kernel patch:
>> https://patchwork.kernel.org/project/netdevbpf/patch/
>> 20211118041501.3102861-8-kuba@kernel.org/
>> https://www.spinics.net/lists/netdev/msg764992.html
>>
>> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>> v3:
>> * fix compiling errors.
>>
>> v2:
>> * change 'memcpy' to 'ether_addr_copy' to copy device addr.
>> ---
>>   kernel/linux/kni/kni_misc.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
>> index 780187d8bf..9c5d8c5576 100644
>> --- a/kernel/linux/kni/kni_misc.c
>> +++ b/kernel/linux/kni/kni_misc.c
>> @@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>>       struct kni_net *knet = net_generic(net, kni_net_id);
>>       int ret;
>>       struct rte_kni_device_info dev_info;
>> +    unsigned char mac_addr[ETH_ALEN];
>>       struct net_device *net_dev = NULL;
>>       struct kni_dev *kni, *dev, *n;
>> @@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t 
>> ioctl_num,
>>       /* if user has provided a valid mac address */
>>       if (is_valid_ether_addr(dev_info.mac_addr))
>> -        memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
>> +        ether_addr_copy(mac_addr, dev_info.mac_addr);
>>       else
>>           /* Generate random MAC address. */
>> -        eth_random_addr(net_dev->dev_addr);
>> +        eth_random_addr(mac_addr);
>> +#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
>> +    ether_addr_copy(net_dev->dev_addr, mac_addr)
> 
> Since semicolon is missing above, it breaks build.
> Please, fix and send v4.
> 
>> +#else
>> +    dev_addr_set(net_dev, mac_addr);
>> +#endif
>>       if (dev_info.mtu)
>>           net_dev->mtu = dev_info.mtu;
> 
> .

Re: [PATCH v4] kni: fix device address set

[Andrew Rybchenko]

On 6/1/22 04:59, Min Hu (Connor) wrote:
> Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
> dmesg buffer get calltrace, info like:
> [ 5965.847401] rte_kni: Creating kni...
> [ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
> [ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
> [ 6225.653010] ------------[ cut here ]------------
> [ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
> [ 6225.832647] Call trace:
> [ 6225.835083]  dev_addr_check+0xa0/0x144
> [ 6225.838816]  dev_addr_flush+0x30/0x9c
> [ 6225.842462]  free_netdev+0x8c/0x1e0
> [ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
> [ 6225.850281]  __fput+0x78/0x220
> [ 6225.853327]  ____fput+0x1c/0x30
> [ 6225.856455]  task_work_run+0x88/0xc0
> [ 6225.860017]  do_exit+0x2fc/0x940
> [ 6225.863232]  do_group_exit+0x40/0xac
> [ 6225.866791]  get_signal+0x190/0x960
> [ 6225.870265]  do_notify_resume+0x26c/0x1360
> [ 6225.874346]  el0_interrupt+0x60/0xe0
> [ 6225.877910]  __el0_irq_handler_common+0x18/0x24
> [ 6225.882420]  el0t_64_irq_handler+0x14/0x20
> [ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
> [ 6225.890059] ---[ end trace 0000000000000000 ]---
> [ 6245.598157] rte_kni: Creating kni...
> 
> The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
> 'dev_addr_set' to ensure that netdev->dev_addr should only be modified
> via helpers('dev_addr_set'). 'dev_addr_check' will check if
> netdev->dev_addr is modified by other ways, like 'memcpy'.
> 
> More info could get by referring to kernel patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/
> 20211118041501.3102861-8-kuba@kernel.org/
> https://www.spinics.net/lists/netdev/msg764992.html
> 
> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v3,v4:
> * fix compiling errors.
> 
> v2:
> * change 'memcpy' to 'ether_addr_copy' to copy device addr.
> ---
>   kernel/linux/kni/kni_misc.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index 780187d8bf..9c5d8c5576 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   	struct kni_net *knet = net_generic(net, kni_net_id);
>   	int ret;
>   	struct rte_kni_device_info dev_info;
> +	unsigned char mac_addr[ETH_ALEN];
>   	struct net_device *net_dev = NULL;
>   	struct kni_dev *kni, *dev, *n;
>   
> @@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   
>   	/* if user has provided a valid mac address */
>   	if (is_valid_ether_addr(dev_info.mac_addr))
> -		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
> +		ether_addr_copy(mac_addr, dev_info.mac_addr);
>   	else
>   		/* Generate random MAC address. */
> -		eth_random_addr(net_dev->dev_addr);
> +		eth_random_addr(mac_addr);

Isn't it better to use function provided by the kernel, if available?
I mean eth_hw_addr_random() used in [1]

[1] 
https://patches.dpdk.org/project/dpdk/patch/20220601054525.7573-1-ke1x.zhang@intel.com/

> +#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE

I think usage of HAVE_* defines like in [1] look much better.
It is easier to read and will limit scope if condition is finally
more complicated.

[1] 
https://patches.dpdk.org/project/dpdk/patch/20220525102641.20982-1-jslaby@suse.cz/

> +	ether_addr_copy(net_dev->dev_addr, mac_addr);
> +#else
> +	dev_addr_set(net_dev, mac_addr);
> +#endif
>   
>   	if (dev_info.mtu)
>   		net_dev->mtu = dev_info.mtu;

[PATCH v5] kni: fix device address set

[Min Hu (Connor)]

Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
dmesg buffer get calltrace, info like:
[ 5965.847401] rte_kni: Creating kni...
[ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
[ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
[ 6225.653010] ------------[ cut here ]------------
[ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
[ 6225.832647] Call trace:
[ 6225.835083]  dev_addr_check+0xa0/0x144
[ 6225.838816]  dev_addr_flush+0x30/0x9c
[ 6225.842462]  free_netdev+0x8c/0x1e0
[ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
[ 6225.850281]  __fput+0x78/0x220
[ 6225.853327]  ____fput+0x1c/0x30
[ 6225.856455]  task_work_run+0x88/0xc0
[ 6225.860017]  do_exit+0x2fc/0x940
[ 6225.863232]  do_group_exit+0x40/0xac
[ 6225.866791]  get_signal+0x190/0x960
[ 6225.870265]  do_notify_resume+0x26c/0x1360
[ 6225.874346]  el0_interrupt+0x60/0xe0
[ 6225.877910]  __el0_irq_handler_common+0x18/0x24
[ 6225.882420]  el0t_64_irq_handler+0x14/0x20
[ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
[ 6225.890059] ---[ end trace 0000000000000000 ]---
[ 6245.598157] rte_kni: Creating kni...

The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
'dev_addr_set' to ensure that netdev->dev_addr should only be modified
via helpers('dev_addr_set'). 'dev_addr_check' will check if
netdev->dev_addr is modified by other ways, like 'memcpy'.

More info could get by referring to kernel patch:
https://patchwork.kernel.org/project/netdevbpf/patch/
20211118041501.3102861-8-kuba@kernel.org/
https://www.spinics.net/lists/netdev/msg764992.html

Fixes: ea6b39b5b847 ("kni: remove ethtool support")

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v5:
* define 'HAVE_DEV_ADDR_SHADOW'.

v3,v4:
* fix compiling errors.

v2:
* change 'memcpy' to 'ether_addr_copy' to copy device addr.
---
 kernel/linux/kni/compat.h   |  4 ++++
 kernel/linux/kni/kni_misc.c | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/kernel/linux/kni/compat.h b/kernel/linux/kni/compat.h
index 664785674f..7ffca63bd4 100644
--- a/kernel/linux/kni/compat.h
+++ b/kernel/linux/kni/compat.h
@@ -141,3 +141,7 @@
 #if KERNEL_VERSION(5, 9, 0) > LINUX_VERSION_CODE
 #define HAVE_TSK_IN_GUP
 #endif
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 17, 0)
+#define HAVE_DEV_ADDR_SHADOW
+#endif
diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
index 780187d8bf..0470d349c5 100644
--- a/kernel/linux/kni/kni_misc.c
+++ b/kernel/linux/kni/kni_misc.c
@@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 	struct kni_net *knet = net_generic(net, kni_net_id);
 	int ret;
 	struct rte_kni_device_info dev_info;
+	unsigned char mac_addr[ETH_ALEN];
 	struct net_device *net_dev = NULL;
 	struct kni_dev *kni, *dev, *n;
 
@@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
 
 	/* if user has provided a valid mac address */
 	if (is_valid_ether_addr(dev_info.mac_addr))
-		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
+		ether_addr_copy(mac_addr, dev_info.mac_addr);
 	else
 		/* Generate random MAC address. */
-		eth_random_addr(net_dev->dev_addr);
+		eth_random_addr(mac_addr);
+#ifdef HAVE_DEV_ADDR_SHADOW
+	dev_addr_set(net_dev, mac_addr);
+#else
+	ether_addr_copy(net_dev->dev_addr, mac_addr);
+#endif
 
 	if (dev_info.mtu)
 		net_dev->mtu = dev_info.mtu;
-- 
2.33.0

Re: [PATCH v4] kni: fix device address set

[Min Hu (Connor)]

Hi, Andrew,

在 2022/6/1 17:02, Andrew Rybchenko 写道:
> On 6/1/22 04:59, Min Hu (Connor) wrote:
>> Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
>> dmesg buffer get calltrace, info like:
>> [ 5965.847401] rte_kni: Creating kni...
>> [ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 
>> 00..
>> [ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 
>> 00..
>> [ 6225.653010] ------------[ cut here ]------------
>> [ 6225.657609] netdevice: vEth0 (unregistered): Incorrect 
>> netdev->dev_addr
>> [ 6225.832647] Call trace:
>> [ 6225.835083]  dev_addr_check+0xa0/0x144
>> [ 6225.838816]  dev_addr_flush+0x30/0x9c
>> [ 6225.842462]  free_netdev+0x8c/0x1e0
>> [ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
>> [ 6225.850281]  __fput+0x78/0x220
>> [ 6225.853327]  ____fput+0x1c/0x30
>> [ 6225.856455]  task_work_run+0x88/0xc0
>> [ 6225.860017]  do_exit+0x2fc/0x940
>> [ 6225.863232]  do_group_exit+0x40/0xac
>> [ 6225.866791]  get_signal+0x190/0x960
>> [ 6225.870265]  do_notify_resume+0x26c/0x1360
>> [ 6225.874346]  el0_interrupt+0x60/0xe0
>> [ 6225.877910]  __el0_irq_handler_common+0x18/0x24
>> [ 6225.882420]  el0t_64_irq_handler+0x14/0x20
>> [ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
>> [ 6225.890059] ---[ end trace 0000000000000000 ]---
>> [ 6245.598157] rte_kni: Creating kni...
>>
>> The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
>> 'dev_addr_set' to ensure that netdev->dev_addr should only be modified
>> via helpers('dev_addr_set'). 'dev_addr_check' will check if
>> netdev->dev_addr is modified by other ways, like 'memcpy'.
>>
>> More info could get by referring to kernel patch:
>> https://patchwork.kernel.org/project/netdevbpf/patch/
>> 20211118041501.3102861-8-kuba@kernel.org/
>> https://www.spinics.net/lists/netdev/msg764992.html
>>
>> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>> v3,v4:
>> * fix compiling errors.
>>
>> v2:
>> * change 'memcpy' to 'ether_addr_copy' to copy device addr.
>> ---
>>   kernel/linux/kni/kni_misc.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
>> index 780187d8bf..9c5d8c5576 100644
>> --- a/kernel/linux/kni/kni_misc.c
>> +++ b/kernel/linux/kni/kni_misc.c
>> @@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>>       struct kni_net *knet = net_generic(net, kni_net_id);
>>       int ret;
>>       struct rte_kni_device_info dev_info;
>> +    unsigned char mac_addr[ETH_ALEN];
>>       struct net_device *net_dev = NULL;
>>       struct kni_dev *kni, *dev, *n;
>> @@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t 
>> ioctl_num,
>>       /* if user has provided a valid mac address */
>>       if (is_valid_ether_addr(dev_info.mac_addr))
>> -        memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
>> +        ether_addr_copy(mac_addr, dev_info.mac_addr);
>>       else
>>           /* Generate random MAC address. */
>> -        eth_random_addr(net_dev->dev_addr);
>> +        eth_random_addr(mac_addr);
> 
> Isn't it better to use function provided by the kernel, if available?
> I mean eth_hw_addr_random() used in [1]
Well, here 'mac_addr' is my object, this object will be used in
'dev_addr_set' in the next code.
using eth_hw_addr_random will directly operate net-dev->dev_addr.

> 
> [1] 
> https://patches.dpdk.org/project/dpdk/patch/20220601054525.7573-1-ke1x.zhang@intel.com/ 
> 
> 
>> +#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
> 
> I think usage of HAVE_* defines like in [1] look much better.
> It is easier to read and will limit scope if condition is finally
> more complicated.
Yes, fixed in v5.
> 
> [1] 
> https://patches.dpdk.org/project/dpdk/patch/20220525102641.20982-1-jslaby@suse.cz/ 
> 
> 
>> +    ether_addr_copy(net_dev->dev_addr, mac_addr);
>> +#else
>> +    dev_addr_set(net_dev, mac_addr);
>> +#endif
>>       if (dev_info.mtu)
>>           net_dev->mtu = dev_info.mtu;
> 
> .

Re: [PATCH] kni: fix device address set

[Stephen Hemminger]

On Wed, 6 Apr 2022 16:22:13 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
> dmesg buffer get calltrace, info like:
> [ 5965.847401] rte_kni: Creating kni...
> [ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
> [ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
> [ 6225.653010] ------------[ cut here ]------------
> [ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
> [ 6225.832647] Call trace:
> [ 6225.835083]  dev_addr_check+0xa0/0x144
> [ 6225.838816]  dev_addr_flush+0x30/0x9c
> [ 6225.842462]  free_netdev+0x8c/0x1e0
> [ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
> [ 6225.850281]  __fput+0x78/0x220
> [ 6225.853327]  ____fput+0x1c/0x30
> [ 6225.856455]  task_work_run+0x88/0xc0
> [ 6225.860017]  do_exit+0x2fc/0x940
> [ 6225.863232]  do_group_exit+0x40/0xac
> [ 6225.866791]  get_signal+0x190/0x960
> [ 6225.870265]  do_notify_resume+0x26c/0x1360
> [ 6225.874346]  el0_interrupt+0x60/0xe0
> [ 6225.877910]  __el0_irq_handler_common+0x18/0x24
> [ 6225.882420]  el0t_64_irq_handler+0x14/0x20
> [ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
> [ 6225.890059] ---[ end trace 0000000000000000 ]---
> [ 6245.598157] rte_kni: Creating kni...
> 
> The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
> 'dev_addr_set' to ensure that netdev->dev_addr should only be modified
> via helpers('dev_addr_set'). 'dev_addr_check' will check if
> netdev->dev_addr is modified by other ways, like 'memcpy'.
> 
> More info could get by referring to kernel patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/
> 20211118041501.3102861-8-kuba@kernel.org/
> https://www.spinics.net/lists/netdev/msg764992.html
> 
> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  kernel/linux/kni/kni_misc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index 780187d8bf..046de0311f 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>  	struct kni_net *knet = net_generic(net, kni_net_id);
>  	int ret;
>  	struct rte_kni_device_info dev_info;
> +	unsigned char mac_addr[ETH_ALEN];
>  	struct net_device *net_dev = NULL;
>  	struct kni_dev *kni, *dev, *n;
>  
> @@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>  
>  	/* if user has provided a valid mac address */
>  	if (is_valid_ether_addr(dev_info.mac_addr))
> -		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
> +		memcpy(mac_addr, dev_info.mac_addr, ETH_ALEN);
>  	else
>  		/* Generate random MAC address. */
> -		eth_random_addr(net_dev->dev_addr);
> +		eth_random_addr(mac_addr);
> +#if KERNEL_VERSION(5, 17, 0) > LINUX_VERSION_CODE
> +	memcpy(net_dev->dev_addr, mac_addr, ETH_ALEN);
> +#else
> +	dev_addr_set(net_dev, mac_addr);
> +#endif

This is still not the best way to handle this.
If you use eth_hw_addr_random() it will set the address assign type.
Right now KNI is not doing this correctly.

Also, the code is more readable if all the kernel compatibility
stuff is handled in compat.h. There should be no kernel version
checks in the functions.

Re: [PATCH v5] kni: fix device address set

[Andrew Rybchenko]

On 6/2/22 09:54, Min Hu (Connor) wrote:
> Currently, run KNI APP When Kernel version is 5.17. When quit the APP,
> dmesg buffer get calltrace, info like:
> [ 5965.847401] rte_kni: Creating kni...
> [ 6225.627205] vEth0 (unregistered): Current addr:  70 fd 45 d0 72 a7 00..
> [ 6225.640113] vEth0 (unregistered): Expected addr: 00 00 00 00 00 00 00..
> [ 6225.653010] ------------[ cut here ]------------
> [ 6225.657609] netdevice: vEth0 (unregistered): Incorrect netdev->dev_addr
> [ 6225.832647] Call trace:
> [ 6225.835083]  dev_addr_check+0xa0/0x144
> [ 6225.838816]  dev_addr_flush+0x30/0x9c
> [ 6225.842462]  free_netdev+0x8c/0x1e0
> [ 6225.845939]  kni_release+0xc0/0x1d0 [rte_kni]
> [ 6225.850281]  __fput+0x78/0x220
> [ 6225.853327]  ____fput+0x1c/0x30
> [ 6225.856455]  task_work_run+0x88/0xc0
> [ 6225.860017]  do_exit+0x2fc/0x940
> [ 6225.863232]  do_group_exit+0x40/0xac
> [ 6225.866791]  get_signal+0x190/0x960
> [ 6225.870265]  do_notify_resume+0x26c/0x1360
> [ 6225.874346]  el0_interrupt+0x60/0xe0
> [ 6225.877910]  __el0_irq_handler_common+0x18/0x24
> [ 6225.882420]  el0t_64_irq_handler+0x14/0x20
> [ 6225.886499]  el0t_64_irq+0x1a0/0x1a4
> [ 6225.890059] ---[ end trace 0000000000000000 ]---
> [ 6245.598157] rte_kni: Creating kni...
> 
> The reason is that 5.17 kernel introduce 'dev_addr_shadow' in function
> 'dev_addr_set' to ensure that netdev->dev_addr should only be modified
> via helpers('dev_addr_set'). 'dev_addr_check' will check if
> netdev->dev_addr is modified by other ways, like 'memcpy'.
> 
> More info could get by referring to kernel patch:
> https://patchwork.kernel.org/project/netdevbpf/patch/
> 20211118041501.3102861-8-kuba@kernel.org/
> https://www.spinics.net/lists/netdev/msg764992.html
> 
> Fixes: ea6b39b5b847 ("kni: remove ethtool support")
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v5:
> * define 'HAVE_DEV_ADDR_SHADOW'.
> 
> v3,v4:
> * fix compiling errors.
> 
> v2:
> * change 'memcpy' to 'ether_addr_copy' to copy device addr.
> ---
>   kernel/linux/kni/compat.h   |  4 ++++
>   kernel/linux/kni/kni_misc.c | 10 ++++++++--
>   2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/linux/kni/compat.h b/kernel/linux/kni/compat.h
> index 664785674f..7ffca63bd4 100644
> --- a/kernel/linux/kni/compat.h
> +++ b/kernel/linux/kni/compat.h
> @@ -141,3 +141,7 @@
>   #if KERNEL_VERSION(5, 9, 0) > LINUX_VERSION_CODE
>   #define HAVE_TSK_IN_GUP
>   #endif
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(5, 17, 0)
> +#define HAVE_DEV_ADDR_SHADOW
> +#endif
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index 780187d8bf..0470d349c5 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -299,6 +299,7 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   	struct kni_net *knet = net_generic(net, kni_net_id);
>   	int ret;
>   	struct rte_kni_device_info dev_info;
> +	unsigned char mac_addr[ETH_ALEN];
>   	struct net_device *net_dev = NULL;
>   	struct kni_dev *kni, *dev, *n;
>   
> @@ -403,10 +404,15 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>   
>   	/* if user has provided a valid mac address */
>   	if (is_valid_ether_addr(dev_info.mac_addr))
> -		memcpy(net_dev->dev_addr, dev_info.mac_addr, ETH_ALEN);
> +		ether_addr_copy(mac_addr, dev_info.mac_addr);
>   	else
>   		/* Generate random MAC address. */
> -		eth_random_addr(net_dev->dev_addr);
> +		eth_random_addr(mac_addr);

The following point from Stephen is still not processed:

<quote>
This is still not the best way to handle this.
If you use eth_hw_addr_random() it will set the address assign type. 
Right now KNI is not doing this correctly.
</quote>

> +#ifdef HAVE_DEV_ADDR_SHADOW
> +	dev_addr_set(net_dev, mac_addr);
> +#else
> +	ether_addr_copy(net_dev->dev_addr, mac_addr);
> +#endif
>   
>   	if (dev_info.mtu)
>   		net_dev->mtu = dev_info.mtu;