[PATCH] net/virtio: include ipv4 cksum to support cksum offload capability

2022-01-07 Thread Harold Huang 
Device cksum offload capability usually include ipv4 cksum, tcp and udp
cksum offload capability. The application such as OVS usually negotiate
with the drive like this to enable cksum offload.

Signed-off-by: Harold Huang 
---
 drivers/net/virtio/virtio_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c 
b/drivers/net/virtio/virtio_ethdev.c
index c2588369b2..65b03bf0e4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -3041,6 +3041,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_SCATTER;
if (host_features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
dev_info->rx_offload_capa |=
+   RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_UDP_CKSUM;
}
@@ -3055,6 +3056,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
if (host_features & (1ULL << VIRTIO_NET_F_CSUM)) {
dev_info->tx_offload_capa |=
+   RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |
RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
RTE_ETH_TX_OFFLOAD_TCP_CKSUM;
}
-- 
2.27.0

Re: [PATCH] net/virtio: include ipv4 cksum to support cksum offload capability

2022-02-06 Thread Harold Huang 
Hi, Maxime,

Maxime Coquelin  于2022年1月31日周一 18:04写道：
>
> Hi Harold,
>
> On 1/7/22 12:53, Harold Huang wrote:
> > Device cksum offload capability usually include ipv4 cksum, tcp and udp
> > cksum offload capability. The application such as OVS usually negotiate
> > with the drive like this to enable cksum offload.
> >
> > Signed-off-by: Harold Huang 
> > ---
> >   drivers/net/virtio/virtio_ethdev.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c 
> > b/drivers/net/virtio/virtio_ethdev.c
> > index c2588369b2..65b03bf0e4 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -3041,6 +3041,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct 
> > rte_eth_dev_info *dev_info)
> >   dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_SCATTER;
> >   if (host_features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
> >   dev_info->rx_offload_capa |=
> > + RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |
> >   RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
> >   RTE_ETH_RX_OFFLOAD_UDP_CKSUM;
> >   }
> > @@ -3055,6 +3056,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct 
> > rte_eth_dev_info *dev_info)
> >   RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
> >   if (host_features & (1ULL << VIRTIO_NET_F_CSUM)) {
> >   dev_info->tx_offload_capa |=
> > + RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |
> >   RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
> >   RTE_ETH_TX_OFFLOAD_TCP_CKSUM;
> >   }
>
> I'm not sure to understand why this is needed, as Vhost lib will always
> ensure the IP csum has been calculated. Could you please elaborate?

Thanks for your comments. Previously I want to enable tx checksum
offload for the tap device when I use DPDK virtio-user driver with
OVS. OVS assume checksum offload capability includes tcp, udp and ipv4
checksum offload:
https://github.com/openvswitch/ovs/blob/master/lib/netdev-dpdk.c#L1097.
But  AFAIK,  ipv4 checksum has always been calculated in the kernel.
And according to the virito-spec
(https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html),
VIRTIO_NET_F_GUEST_CSUM feature bit may only indicate TCP or UDP
checksum offload, right? Maybe I need to change OVS to adapt to
virtio-user driver to enable checksum offload.

>
> Thanks,
> Maxime
>

Thanks,
Harold

[PATCH] net/kni: reset rte_kni_conf struct before initialization

2021-12-02 Thread Harold Huang 
When kni driver calls eth_kni_start to start device, some fields such
as min_mtu and max_mtu of rte_kni_conf are not initialized. It will
cause kni_ioctl_create create a kni netdevice with a random min_mtu
and max_mtu value. This is unexpected and in some time we could not
change the kni device mtu with ip link command.

Fixes: ff1e35fb5f8 ("kni: calculate MTU from mbuf size")
Signed-off-by: Harold Huang 
---
 drivers/net/kni/rte_eth_kni.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index c428caf441..23b15edfac 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -128,6 +128,7 @@ eth_kni_start(struct rte_eth_dev *dev)
  const char *name = dev->device->name + 4; /* remove net_ */

  mb_pool = internals->rx_queues[0].mb_pool;
+ memset(&conf, 0, sizeof(conf));
  strlcpy(conf.name, name, RTE_KNI_NAMESIZE);
  conf.force_bind = 0;
  conf.group_id = port_id;
--
2.18.2

[PATCH v2] net/kni: reset rte_kni_conf struct before initialization

2021-12-04 Thread Harold Huang 
When kni driver calls eth_kni_start to start device, some fields such as 
min_mtu and max_mtu of rte_kni_conf are not initialized. It will cause 
kni_ioctl_create create a kni netdevice with a random min_mtu and max_mtu 
value. This is unexpected and in some time we could not change the kni device 
mtu with ip link command.

Fixes: ff1e35fb5f8 ("kni: calculate MTU from mbuf size")
Signed-off-by: Harold Huang 
---
 drivers/net/kni/rte_eth_kni.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index c428caf441..23b15edfac 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -128,6 +128,7 @@ eth_kni_start(struct rte_eth_dev *dev)
const char *name = dev->device->name + 4; /* remove net_ */
 
mb_pool = internals->rx_queues[0].mb_pool;
+   memset(&conf, 0, sizeof(conf));
strlcpy(conf.name, name, RTE_KNI_NAMESIZE);
conf.force_bind = 0;
conf.group_id = port_id;
-- 
2.27.0

[PATCH v3] net/kni: reset rte_kni_conf struct before initialization

2021-12-04 Thread Harold Huang 
When kni driver calls eth_kni_start to start device, some fields such as
min_mtu and max_mtu of rte_kni_conf are not initialized. It will cause
kni_ioctl_create create a kni netdevice with a random min_mtu and max_mtu
value. This isunexpected and in some time we could not change the kni
device mtu with ip link command.

Fixes: ff1e35fb5f8 ("kni: calculate MTU from mbuf size")
Signed-off-by: Harold Huang 
---
 drivers/net/kni/rte_eth_kni.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index c428caf441..23b15edfac 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -128,6 +128,7 @@ eth_kni_start(struct rte_eth_dev *dev)
const char *name = dev->device->name + 4; /* remove net_ */
 
mb_pool = internals->rx_queues[0].mb_pool;
+   memset(&conf, 0, sizeof(conf));
strlcpy(conf.name, name, RTE_KNI_NAMESIZE);
conf.force_bind = 0;
conf.group_id = port_id;
-- 
2.27.0

[PATCH v4] net/kni: reset rte_kni_conf struct before initialization

2021-12-04 Thread Harold Huang 
When kni driver calls eth_kni_start to start device, some fields such as
min_mtu and max_mtu of rte_kni_conf are not initialized. It will cause
kni_ioctl_create create a kni netdevice with a random min_mtu and max_mtu
value. This isunexpected and in some time we could not change the kni
device mtu with ip link command.

Fixes: ff1e35fb5f8 ("kni: calculate MTU from mbuf size")
Signed-off-by: Harold Huang 
---
 drivers/net/kni/rte_eth_kni.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index c428caf441..23b15edfac 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -128,6 +128,7 @@ eth_kni_start(struct rte_eth_dev *dev)
const char *name = dev->device->name + 4; /* remove net_ */
 
mb_pool = internals->rx_queues[0].mb_pool;
+   memset(&conf, 0, sizeof(conf));
strlcpy(conf.name, name, RTE_KNI_NAMESIZE);
conf.force_bind = 0;
conf.group_id = port_id;
-- 
2.27.0

[PATCH] net/virtio: unregister virtio user memery event to fix memory leak problem

2021-12-20 Thread Harold Huang 
When eth_virtio_dev_init is failed, the registered virtio user memery is
not released and creating a new virtio user dev could be failed becase the
new dev could use the same address pointer and the register virtio user
memory to the same address is not allowed.

Signed-off-by: Harold Huang 
---
 drivers/net/virtio/virtio_user_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c 
b/drivers/net/virtio/virtio_user_ethdev.c
index 0271098f0d..16eca2f940 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -666,6 +666,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
/* previously called by pci probing for physical dev */
if (eth_virtio_dev_init(eth_dev) < 0) {
PMD_INIT_LOG(ERR, "eth_virtio_dev_init fails");
+   virtio_user_dev_uninit(dev);
virtio_user_eth_dev_free(eth_dev);
goto end;
}
-- 
2.27.0

[PATCH v2] net/virtio: unregister virtio user memory event to fix memory leak problem

2021-12-20 Thread Harold Huang 
When eth_virtio_dev_init is failed, the registered virtio user memory is
not released and creating a new virtio user dev could be failed because the
new dev could use the same address pointer and the register virtio user
memory to the same address is not allowed.

Signed-off-by: Harold Huang 
---
 drivers/net/virtio/virtio_user_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c 
b/drivers/net/virtio/virtio_user_ethdev.c
index 0271098f0d..16eca2f940 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -666,6 +666,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
/* previously called by pci probing for physical dev */
if (eth_virtio_dev_init(eth_dev) < 0) {
PMD_INIT_LOG(ERR, "eth_virtio_dev_init fails");
+   virtio_user_dev_uninit(dev);
virtio_user_eth_dev_free(eth_dev);
goto end;
}
-- 
2.27.0

Fwd: [PATCH v2] net/virtio: unregister virtio user memory event to fix memory leak problem

2021-12-20 Thread Harold Huang 
The problem which this patch wants to solve can be reproduced via ovs 2.16:

1. add a port with an invalid speed parameter:
ovs-vsctl add-port ovs-br0 virtiouser0 -- set Interface virtiouser0
type=dpdk 
options:dpdk-devargs=virtio_user0,path=/dev/vhost-net,queue_size=1024,queues=1,speed=1000

2. delete the failed virtiouser0 port:
ovs-vsctl del-port virtiouser0

3. add a new port with valid parameters:
ovs-vsctl add-port ovs-br0 virtiouser0 -- set Interface virtiouser0
type=dpdk 
options:dpdk-devargs=virtio_user0,path=/dev/vhost-net,queue_size=1024,queues=1

The newly added port is always failed because of an existing memory event error.

[PATCH v3] net/virtio: fix unregister virtio user memory event cb problem

2021-12-22 Thread Harold Huang 
When eth_virtio_dev_init is failed, the registered virtio user memory
event cb is not released and creating a new vdev could be failed
because the new virtio_user_dev could use the same address pointer and
register memory event cb to the same address is not allowed.

Signed-off-by: Harold Huang 
---
Compared PATCH v2, commit message is changed. The problem this patch want
to solve can be reproduced by ovs 2.16.90 and the latest dpdk.
 drivers/net/virtio/virtio_user_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c 
b/drivers/net/virtio/virtio_user_ethdev.c
index 0271098f0d..16eca2f940 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -666,6 +666,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
/* previously called by pci probing for physical dev */
if (eth_virtio_dev_init(eth_dev) < 0) {
PMD_INIT_LOG(ERR, "eth_virtio_dev_init fails");
+   virtio_user_dev_uninit(dev);
virtio_user_eth_dev_free(eth_dev);
goto end;
}
-- 
2.27.0

Re: [PATCH v4] net/kni: reset rte_kni_conf struct before initialization

2021-12-22 Thread Harold Huang 
The problem this patch wants to solve can be reproduced by ovs-dpdk:

1. create a kni  port in ovs:
ovs-vsctl add-port ovs-br0 kni0 -- set Interface kni0 type=dpdk
options:dpdk-devargs=net_kni0

2. change the mtu of the kni device in kernel:
ip link set kni0  mtu 1500

If we do not set rte_kni_conf to 0, the min_mtu and max_mtu are random
and change mtu could be failed.

[PATCH] net/virtio: fix unreleased resource when creating virtio user dev is failed

2021-12-22 Thread Harold Huang 
When eth_virtio_dev_init is failed, the registered virtio user memory event
cb is not released and the backend created tap device is not destroyed.
It would cause some residual tap device existed in the host and creating
a new vdev could be failed because the new virtio_user_dev could use the
same address pointer and register memory event cb to the same address is
not allowed.

Signed-off-by: Harold Huang 
---
Compared to patch v3, commit log is changed because this bug could
cause residual tap device in the host.
 drivers/net/virtio/virtio_user_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio/virtio_user_ethdev.c 
b/drivers/net/virtio/virtio_user_ethdev.c
index 0271098f0d..16eca2f940 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -666,6 +666,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
/* previously called by pci probing for physical dev */
if (eth_virtio_dev_init(eth_dev) < 0) {
PMD_INIT_LOG(ERR, "eth_virtio_dev_init fails");
+   virtio_user_dev_uninit(dev);
virtio_user_eth_dev_free(eth_dev);
goto end;
}
-- 
2.27.0

[PATCH] app/flow-perf: replace RTE_BE32/16 with rte_cpu_to_be_32/16 for variables

2023-03-11 Thread Harold Huang 
In DPDK, the macros RTE_BE32 or RTE_BE16 are usually used for
constant values. And functions such as rte_cpu_to_be_32 or
rte_cpu_to_be_16 are optimized for variables.

Signed-off-by: Harold Huang 
---
 app/test-flow-perf/actions_gen.c | 28 ++--
 app/test-flow-perf/items_gen.c   |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/app/test-flow-perf/actions_gen.c b/app/test-flow-perf/actions_gen.c
index f1d5931325..c2499ad2d0 100644
--- a/app/test-flow-perf/actions_gen.c
+++ b/app/test-flow-perf/actions_gen.c
@@ -262,7 +262,7 @@ add_set_src_ipv4(struct rte_flow_action *actions,
ip = 1;
 
/* IPv4 value to be set is random each time */
-   set_ipv4[para.core_idx].ipv4_addr = RTE_BE32(ip + 1);
+   set_ipv4[para.core_idx].ipv4_addr = rte_cpu_to_be_32(ip + 1);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC;
actions[actions_counter].conf = &set_ipv4[para.core_idx];
@@ -281,7 +281,7 @@ add_set_dst_ipv4(struct rte_flow_action *actions,
ip = 1;
 
/* IPv4 value to be set is random each time */
-   set_ipv4[para.core_idx].ipv4_addr = RTE_BE32(ip + 1);
+   set_ipv4[para.core_idx].ipv4_addr = rte_cpu_to_be_32(ip + 1);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_SET_IPV4_DST;
actions[actions_counter].conf = &set_ipv4[para.core_idx];
@@ -348,7 +348,7 @@ add_set_src_tp(struct rte_flow_action *actions,
/* TP src port is random each time */
tp = tp % 0x;
 
-   set_tp[para.core_idx].port = RTE_BE16(tp & 0x);
+   set_tp[para.core_idx].port = rte_cpu_to_be_16(tp & 0x);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_SET_TP_SRC;
actions[actions_counter].conf = &set_tp[para.core_idx];
@@ -370,7 +370,7 @@ add_set_dst_tp(struct rte_flow_action *actions,
if (tp > 0x)
tp = tp >> 16;
 
-   set_tp[para.core_idx].port = RTE_BE16(tp & 0x);
+   set_tp[para.core_idx].port = rte_cpu_to_be_16(tp & 0x);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_SET_TP_DST;
actions[actions_counter].conf = &set_tp[para.core_idx];
@@ -388,7 +388,7 @@ add_inc_tcp_ack(struct rte_flow_action *actions,
if (!para.unique_data)
ack_value = 1;
 
-   value[para.core_idx] = RTE_BE32(ack_value);
+   value[para.core_idx] = rte_cpu_to_be_32(ack_value);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_INC_TCP_ACK;
actions[actions_counter].conf = &value[para.core_idx];
@@ -406,7 +406,7 @@ add_dec_tcp_ack(struct rte_flow_action *actions,
if (!para.unique_data)
ack_value = 1;
 
-   value[para.core_idx] = RTE_BE32(ack_value);
+   value[para.core_idx] = rte_cpu_to_be_32(ack_value);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK;
actions[actions_counter].conf = &value[para.core_idx];
@@ -424,7 +424,7 @@ add_inc_tcp_seq(struct rte_flow_action *actions,
if (!para.unique_data)
seq_value = 1;
 
-   value[para.core_idx] = RTE_BE32(seq_value);
+   value[para.core_idx] = rte_cpu_to_be_32(seq_value);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ;
actions[actions_counter].conf = &value[para.core_idx];
@@ -442,7 +442,7 @@ add_dec_tcp_seq(struct rte_flow_action *actions,
if (!para.unique_data)
seq_value = 1;
 
-   value[para.core_idx] = RTE_BE32(seq_value);
+   value[para.core_idx] = rte_cpu_to_be_32(seq_value);
 
actions[actions_counter].type = RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ;
actions[actions_counter].conf = &value[para.core_idx];
@@ -560,7 +560,7 @@ add_vlan_header(uint8_t **header, uint64_t data,
vlan_value = VLAN_VALUE;
 
memset(&vlan_hdr, 0, sizeof(struct rte_vlan_hdr));
-   vlan_hdr.vlan_tci = RTE_BE16(vlan_value);
+   vlan_hdr.vlan_tci = rte_cpu_to_be_16(vlan_value);
 
if (data & FLOW_ITEM_MASK(RTE_FLOW_ITEM_TYPE_IPV4))
vlan_hdr.eth_proto = RTE_BE16(RTE_ETHER_TYPE_IPV4);
@@ -586,7 +586,7 @@ add_ipv4_header(uint8_t **header, uint64_t data,
 
memset(&ipv4_hdr, 0, sizeof(struct rte_ipv4_hdr));
ipv4_hdr.src_addr = RTE_IPV4(127, 0, 0, 1);
-   ipv4_hdr.dst_addr = RTE_BE32(ip_dst);
+   ipv4_hdr.dst_addr = rte_cpu_to_be_32(ip_dst);
ipv4_hdr.version_ihl = RTE_IPV4_VHL_DEF;
if (data & FLOW_ITEM_MASK(RTE_FLOW_ITEM_TYPE_UDP))
ipv4_hdr.next_proto_id = RTE_IP_TYPE_UDP;
@@ -652,7 +652,7 @@ add_vxlan_header(uint8_t **header, uint64_t data,
 
memset(&vxlan_hdr, 0, sizeof(struct rte_vxlan_hdr));
 
-   vxlan_hdr.vx_vni = (RTE_BE32(vni_value)) >> 16;
+   vxlan_hdr.vx_vni = (rte_cpu_to_be_32(vni_value)) >> 16;
vxlan_hdr.vx_flags = 0x8;
 
memcpy(

Re: [PATCH] app/flow-perf: replace RTE_BE32/16 with rte_cpu_to_be_32/16 for variables

2023-03-27 Thread Harold Huang 
HI,
  I see all the other RTE_BE32 and RTE_BE16 are used for constant
values. I think it is not necessary to fix them:

In app/test-flow-perf/items_gen.c:
.hdr.vlan_tci = RTE_BE16(VLAN_VALUE),
.hdr.vlan_tci = RTE_BE16(0x),
ipv4_masks[ti].hdr.src_addr = RTE_BE32(0x);
.protocol = RTE_BE16(RTE_ETHER_TYPE_TEB),
.protocol = RTE_BE16(0x),
.hdr.teid = RTE_BE32(TEID_VALUE),
.hdr.teid = RTE_BE32(0x),
.data = RTE_BE32(META_DATA),
.data = RTE_BE32(0x),
.data = RTE_BE32(META_DATA),
.data = RTE_BE32(0x),

In app/test-flow-perf/actions_gen.c:
.data = RTE_BE32(META_DATA),
.mask = RTE_BE32(0x),
.data = RTE_BE32(META_DATA),
.mask = RTE_BE32(0x),
eth_hdr.ether_type = RTE_BE16(RTE_ETHER_TYPE_VLAN);
eth_hdr.ether_type = RTE_BE16(RTE_ETHER_TYPE_IPV4);
eth_hdr.ether_type = RTE_BE16(RTE_ETHER_TYPE_IPV6);
vlan_hdr.eth_proto = RTE_BE16(RTE_ETHER_TYPE_IPV4);
vlan_hdr.eth_proto = RTE_BE16(RTE_ETHER_TYPE_IPV6);
udp_hdr.dst_port = RTE_BE16(RTE_VXLAN_DEFAULT_PORT);
udp_hdr.dst_port = RTE_BE16(RTE_VXLAN_GPE_UDP_PORT);
udp_hdr.dst_port = RTE_BE16(RTE_GENEVE_UDP_PORT);
udp_hdr.dst_port = RTE_BE16(RTE_GTPU_UDP_PORT);
gre_hdr.proto = RTE_BE16(RTE_ETHER_TYPE_TEB);
item_udp.hdr.dst_port = RTE_BE16(RTE_VXLAN_DEFAULT_PORT);

On Mon, Mar 27, 2023 at 6:29 PM Wisam Monther  wrote:
>
> Hi,
>
> > -Original Message-
> > From: Harold Huang 
> > Sent: Sunday, March 12, 2023 4:00 AM
> > To: dev@dpdk.org
> > Cc: Harold Huang ; Wisam Monther
> > 
> > Subject: [PATCH] app/flow-perf: replace RTE_BE32/16 with
> > rte_cpu_to_be_32/16 for variables
> >
> > In DPDK, the macros RTE_BE32 or RTE_BE16 are usually used for constant
> > values. And functions such as rte_cpu_to_be_32 or
> > rte_cpu_to_be_16 are optimized for variables.
> >
> > Signed-off-by: Harold Huang 
> > ---
> >  app/test-flow-perf/actions_gen.c | 28 ++--
> >  app/test-flow-perf/items_gen.c   |  2 +-
> >  2 files changed, 15 insertions(+), 15 deletions(-)
> >
>
> Indeed your change is in the correct files and I agree that it's need to be 
> done,
> But you are not doing it for all RTE_BE32 and RTE_BE16 in the app or the same 
> files
>
> After quick search I see:
> app/test-flow-perf/items_gen.c:12
> app/test-flow-perf/actions_gen.c:29
>
> While you are doing the change only for:
> app/test-flow-perf/items_gen.c:1
> app/test-flow-perf/actions_gen.c:14
>
> Can you please extend your fix for all needed vars.
>
> BRs,
> Wisam Jaddo



-- 
Thanks, Harold.

[PATCH] net/tap: do not include l2 header in gso size when compared with mtu

2022-02-28 Thread Harold Huang 
The gso size is calculated with all of the headers and payload. As a
result, the l2 header should not be included when comparing gso size
with mtu.

Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
Cc: sta...@dpdk.org
Signed-off-by: Harold Huang 
---
 drivers/net/tap/rte_eth_tap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index f1b48cae82..2b561d232c 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, uint16_t 
nb_pkts)
mbuf_in->l4_len;
tso_segsz = mbuf_in->tso_segsz + hdrs_len;
if (unlikely(tso_segsz == hdrs_len) ||
-   tso_segsz > *txq->mtu) {
+   tso_segsz > *txq->mtu + mbuf_in->l2_len) {
txq->stats.errs++;
break;
}
-- 
2.27.0

[PATCH] net/virtio: support NAPI when using vhost_net backend

2022-03-02 Thread Harold Huang 
In patch [1], NAPI has been supported in kernel tun driver to accelerate
packet processing received from vhost_net. This will greatly improve the
throughput of the tap device in the vhost_net backend.

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=fb3f903769e8

Signed-off-by: Harold Huang 
---
 drivers/net/virtio/virtio_user/vhost_kernel.c | 9 +++--
 drivers/net/virtio/virtio_user/vhost_kernel_tap.c | 4 ++--
 drivers/net/virtio/virtio_user/vhost_kernel_tap.h | 3 ++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c 
b/drivers/net/virtio/virtio_user/vhost_kernel.c
index 202a8cdee1..986b56a7ca 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
@@ -383,6 +383,7 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
struct vhost_kernel_data *data;
unsigned int tap_features;
unsigned int tap_flags;
+   unsigned int r_flags;
const char *ifname;
uint32_t q, i;
int vhostfd;
@@ -394,6 +395,10 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
PMD_INIT_LOG(ERR, "TAP does not support IFF_VNET_HDR");
return -1;
}
+   r_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
+
+   if (tap_features & IFF_NAPI)
+   r_flags |= IFF_NAPI;
 
data = malloc(sizeof(*data));
if (!data) {
@@ -429,7 +434,7 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
}
 
ifname = dev->ifname != NULL ? dev->ifname : "tap%d";
-   data->tapfds[0] = tap_open(ifname, (tap_features & IFF_MULTI_QUEUE) != 
0);
+   data->tapfds[0] = tap_open(ifname, r_flags, (tap_features & 
IFF_MULTI_QUEUE) != 0);
if (data->tapfds[0] < 0)
goto err_tapfds;
if (dev->ifname == NULL && tap_get_name(data->tapfds[0], &dev->ifname) 
< 0) {
@@ -446,7 +451,7 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
}
 
for (i = 1; i < dev->max_queue_pairs; i++) {
-   data->tapfds[i] = tap_open(dev->ifname, true);
+   data->tapfds[i] = tap_open(dev->ifname, r_flags, true);
if (data->tapfds[i] < 0)
goto err_tapfds;
}
diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c 
b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
index 2aac4028bb..611e2e25ec 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
@@ -42,7 +42,7 @@ tap_support_features(unsigned int *tap_features)
 }
 
 int
-tap_open(const char *ifname, bool multi_queue)
+tap_open(const char *ifname, unsigned int r_flags, bool multi_queue)
 {
struct ifreq ifr;
int tapfd;
@@ -61,7 +61,7 @@ tap_open(const char *ifname, bool multi_queue)
 retry_mono_q:
memset(&ifr, 0, sizeof(ifr));
strncpy(ifr.ifr_name, ifname, IFNAMSIZ - 1);
-   ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
+   ifr.ifr_flags = r_flags;
if (multi_queue)
ifr.ifr_flags |= IFF_MULTI_QUEUE;
if (ioctl(tapfd, TUNSETIFF, (void *)&ifr) == -1) {
diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h 
b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
index 726433c48c..636a0481be 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
+++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
@@ -22,6 +22,7 @@
 #define IFF_NO_PI0x1000
 #define IFF_VNET_HDR 0x4000
 #define IFF_MULTI_QUEUE  0x0100
+#define IFF_NAPI 0x0010
 
 /* Features for GSO (TUNSETOFFLOAD). */
 #define TUN_F_CSUM 0x01/* You can hand me unchecksummed packets. */
@@ -36,7 +37,7 @@
 int vhost_kernel_tap_setup(int tapfd, int hdr_size, uint64_t features);
 
 int tap_support_features(unsigned int *tap_features);
-int tap_open(const char *ifname, bool multi_queue);
+int tap_open(const char *ifname, unsigned int r_flags, bool multi_queue);
 int tap_get_name(int tapfd, char **ifname);
 int tap_get_flags(int tapfd, unsigned int *tap_flags);
 int tap_set_mac(int tapfd, uint8_t *mac);
-- 
2.27.0

[PATCH] net/kni: initialize rte_kni_conf to 0 before using it

2022-03-02 Thread Harold Huang 
When kni driver calls eth_kni_start to start device, some fields such as
min_mtu and max_mtu of rte_kni_conf are not initialized. It will cause
kni_ioctl_create create a kni netdevice with a random min_mtu and max_mtu
value. This is unexpected and in some time we could not change the kni
device mtu with ip link command.

Fixes: ff1e35fb5f8 ("kni: calculate MTU from mbuf size")
Signed-off-by: Harold Huang 
---
 drivers/net/kni/rte_eth_kni.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index c428caf441..23b15edfac 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -128,6 +128,7 @@ eth_kni_start(struct rte_eth_dev *dev)
const char *name = dev->device->name + 4; /* remove net_ */
 
mb_pool = internals->rx_queues[0].mb_pool;
+   memset(&conf, 0, sizeof(conf));
strlcpy(conf.name, name, RTE_KNI_NAMESIZE);
conf.force_bind = 0;
conf.group_id = port_id;
-- 
2.27.0

Re: [PATCH] net/kni: initialize rte_kni_conf to 0 before using it

2022-03-02 Thread Harold Huang 
On Thu, Mar 3, 2022 at 2:16 AM Ferruh Yigit  wrote:
>
> On 3/2/2022 12:33 PM, Harold Huang wrote:
> > When kni driver calls eth_kni_start to start device, some fields such as
> > min_mtu and max_mtu of rte_kni_conf are not initialized. It will cause
> > kni_ioctl_create create a kni netdevice with a random min_mtu and max_mtu
> > value. This is unexpected and in some time we could not change the kni
> > device mtu with ip link command.
> >
>
> Agree on the problem and the solution, thanks for the fix.
>
> > Fixes: ff1e35fb5f8 ("kni: calculate MTU from mbuf size")
> > Signed-off-by: Harold Huang 
> > ---
> >   drivers/net/kni/rte_eth_kni.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
> > index c428caf441..23b15edfac 100644
> > --- a/drivers/net/kni/rte_eth_kni.c
> > +++ b/drivers/net/kni/rte_eth_kni.c
> > @@ -128,6 +128,7 @@ eth_kni_start(struct rte_eth_dev *dev)
> >   const char *name = dev->device->name + 4; /* remove net_ */
> >
> >   mb_pool = internals->rx_queues[0].mb_pool;
> > + memset(&conf, 0, sizeof(conf));
>
> Can you prefer initialize to zero, instead of 'memset', I think it
> is more clear that way:

Thanks. Sounds good,  fix it.

>
>   -   struct rte_kni_conf conf;
>   +   struct rte_kni_conf conf = { 0 };
>
> >   strlcpy(conf.name, name, RTE_KNI_NAMESIZE);
> >   conf.force_bind = 0;
> >   conf.group_id = port_id;
>

[PATCH] net/kni: initialize rte_kni_conf to 0 before using it

2022-03-02 Thread Harold Huang 
When kni driver calls eth_kni_start to start device, some fields such as
min_mtu and max_mtu of rte_kni_conf are not initialized. It will cause
kni_ioctl_create create a kni netdevice with a random min_mtu and max_mtu
value. This is unexpected and sometimes we could not change the kni
device mtu with ip link command.

Fixes: ff1e35fb5f83 ("kni: calculate MTU from mbuf size")
Cc: sta...@dpdk.org
Signed-off-by: Harold Huang 
---
 drivers/net/kni/rte_eth_kni.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
index c428caf441..ba58d9dbae 100644
--- a/drivers/net/kni/rte_eth_kni.c
+++ b/drivers/net/kni/rte_eth_kni.c
@@ -124,7 +124,7 @@ eth_kni_start(struct rte_eth_dev *dev)
struct pmd_internals *internals = dev->data->dev_private;
uint16_t port_id = dev->data->port_id;
struct rte_mempool *mb_pool;
-   struct rte_kni_conf conf;
+   struct rte_kni_conf conf = { 0 };
const char *name = dev->device->name + 4; /* remove net_ */
 
mb_pool = internals->rx_queues[0].mb_pool;
-- 
2.27.0

Re: [PATCH] net/tap: do not include l2 header in gso size when compared with mtu

2022-03-08 Thread Harold Huang 
On Mon, Feb 28, 2022 at 4:27 PM Harold Huang  wrote:
>
> The gso size is calculated with all of the headers and payload. As a
> result, the l2 header should not be included when comparing gso size
> with mtu.
>
> Fixes: 050316a88313 ("net/tap: support TSO (TCP Segment Offload)")
> Cc: sta...@dpdk.org
> Signed-off-by: Harold Huang 
> ---
>  drivers/net/tap/rte_eth_tap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index f1b48cae82..2b561d232c 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -731,7 +731,7 @@ pmd_tx_burst(void *queue, struct rte_mbuf **bufs, 
> uint16_t nb_pkts)
> mbuf_in->l4_len;
> tso_segsz = mbuf_in->tso_segsz + hdrs_len;
> if (unlikely(tso_segsz == hdrs_len) ||
> -   tso_segsz > *txq->mtu) {
> +   tso_segsz > *txq->mtu + mbuf_in->l2_len) {
> txq->stats.errs++;
> break;
> }
> --
> 2.27.0
>

Hi, Jiayu,

This is the only example in the driver to use GSO. I think it is
important for us to calculate a correct GSO size. I see you are the
GSO lib maintainer, could you please help review this patch?

Re: [PATCH 2/6] net/hns3: fix inconsistent enabled RSS behavior

2022-03-24 Thread Harold Huang 
Hi, all,

On Thu, Mar 3, 2022 at 12:59 AM Stephen Hemminger
 wrote:
>
> On Wed, 02 Mar 2022 15:46:37 +0100
> Thomas Monjalon  wrote:
>
> > 02/03/2022 15:07, Ori Kam:
> > > From: lihuisong (C) 
> > > > 在 2022/3/1 0:42, Ferruh Yigit 写道:
> > > > > On 2/28/2022 3:21 AM, Min Hu (Connor) wrote:
> > > > >> From: Huisong Li 
> > > > >>
> > > > >> RSS will not be enabled if the RTE_ETH_MQ_RX_RSS_FLAG isn't be set in
> > > > >> dev_configure phase. However, if this flag isn't set, RSS can be 
> > > > >> enabled
> > > > >> through the ethdev ops and rte_flow API. This behavior is contrary to
> > > > >> each
> > > > >> other.
> > > > >>
> > > > >> Fixes: c37ca66f2b27 ("net/hns3: support RSS")
> > > > >> Cc: sta...@dpdk.org
> > > > >>
> > > > >> Signed-off-by: Huisong Li 
> > > > >
> > > > >
> > > > > Hi Huisong, Connor,
> > > > >
> > > > > Let's get a little more feedback for this patch, cc'ed more people.
> > > > >
> > > > >
> > > > > To enable RSS, multi queue mode should be set to
> > > > > 'RTE_ETH_MQ_RX_RSS_FLAG'.
> > > > >
> > > > > But I wonder if it is required to configure RSS via flow API,
> > > >
> > > > I do not know the original purpose of adding the RSS configuration in
> > > > flow API.

In OVS-DPDK, there is a use case to add mark+rss action via flow API. See [1]:
[1]: 
https://github.com/openvswitch/ovs/blob/master/lib/netdev-offload-dpdk.c#L1677

> > > >
> > > The purpose is simple, this allow to create RSS per rule and not a global 
> > > one.
> > > For example create RSS that sends TCP to some queues while othe RSS will 
> > > send
> > > UDP traffic to different queues.
> > >
> > > > However, as far as I know, the hash algorithm can be configured via this
> > > > API,
> > > >
> > > > but not via ethdev ops API.
> > > >
> > > > > and if other PMDs check this configuration for flow API?
> > > >
> > > > Some PMDs already have similar check in RSS releated ops or rte_flow 
> > > > API.
> > > >
> > > > For example, hinic, axbge, bnxt, cnxk, otx2, and ena.
> >
> > I suggest removing these checks.
> >
> > > From my view point those are two different settings.
> > > The RTE_ETH_MQ_RX_RSS_FLAG is global per port while
> > > rte_flow is per rule.
> > >
> > > I think, that if a PMD needs this flag, in order to enable it also for 
> > > rte_flow then
> > > it should be documented in the release note of the PMD.
> > > It is a valid use case that the default traffic will not have RSS and 
> > > only rules created by
> > > rte_flow will have the RSS, for matching traffc.
> >
> > I think RTE_ETH_MQ_RX_RSS_FLAG should not be required by any PMD
> > for rte_flow RSS rules.
> >
> >
>
> Agreed.
>
> The ideal state around RSS would be:
>- global settings affect the default started queues when device is started.
>  if RSS is enabled and 4 queues are started, then RSS would happen for 
> received
>  packets across those 4 queues.
>- when adding new flow related rx:
>   - new queues could be started - BUT not part of original RSS
>   - new rte_flow rule can do RSS against the new flow:
> example:
>   start queues 4,5,6,7
>   rte_flow match TCP port 80 and process on queues 4,5,6,7 with 
> RSS
>
> This is a mess today, most devices work differently and there is no reference 
> software
> implementation.  In an ideal state, there would be a reference software 
> implementation
> that implements all of rte_flow, and all new hardware support would have to 
> work the same
> as the reference implementation. And there would be a full CI test suite 
> around rte_flow.

When adding a new flow with RSS action, does the RSS hash type have to
be set or it could inherit the global setting? I see there are some
different behaviors when we use RSS actions without hash types. In my
test, the E810 and Broadcom NetXtreme-E work well but there are some
problems in MLX5. There is a patch in [2]. Anyone could give some
suggestions?

[2]: 
https://patchwork.ozlabs.org/project/openvswitch/patch/20220316080842.811478-1-baymaxhu...@gmail.com/

-- 
Thanks, Harold.

Re: [PATCH] kni: update kernel API to receive packets

2022-04-14 Thread Harold Huang 
On Thu, Apr 14, 2022 at 8:23 PM Gagandeep Singh  wrote:
>
> API 'netif_rx_ni()' has been removed in kernel with commit:
> baebdf48c3600 ("net: dev: Makes sure netif_rx() can be invoked in any 
> context.")
>
> The API netif_rx() can be used for any context to receive packets
> from device drivers.
>
> This patch replaces the API netif_rx_ni() with netif_rx().

But this change would cause KNI kernel module does not work in the old
kernel without this patch. I suggested using netif_rx_ni to keep
compatibility.


>
> Signed-off-by: Gagandeep Singh 
> ---
>  kernel/linux/kni/kni_net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
> index 29e5b9e21f..e66b35314a 100644
> --- a/kernel/linux/kni/kni_net.c
> +++ b/kernel/linux/kni/kni_net.c
> @@ -441,7 +441,7 @@ kni_net_rx_normal(struct kni_dev *kni)
> skb->ip_summed = CHECKSUM_UNNECESSARY;
>
> /* Call netif interface */
> -   netif_rx_ni(skb);
> +   netif_rx(skb);
>
> /* Update statistics */
> dev->stats.rx_bytes += len;
> --
> 2.25.1
>


-- 
Thanks, Harold.