Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
Hi Eli,
Thanks for responding. PSB.
On Tue, 6 Jun 2023, Eli Britstein wrote:
-Original Message-
From: Ivan Malov
Sent: Sunday, 4 June 2023 15:58
To: Eli Britstein
Cc: ovs-dev@openvswitch.org; Ilya Maximets ; Ori
Kam ; David Marchand
Subject: RE: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
mechanism
External email: Use caution opening links or attachments
Hi Eli,
Thanks for reviewing this. Please see below.
On Tue, 21 Feb 2023, Eli Britstein wrote:
-Original Message-
From: Ivan Malov
Sent: Tuesday, 21 February 2023 2:41
To: ovs-dev@openvswitch.org
Cc: Ilya Maximets ; Eli Britstein
; Ori Kam ; David Marchand
Subject: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
mechanism
External email: Use caution opening links or attachments
Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified explicitly,
via the corresponding pattern item.
Signed-off-by: Ivan Malov
---
lib/netdev-dpdk.c | 88 +++
lib/netdev-dpdk.h | 4 +-
lib/netdev-offload-dpdk.c | 55 +++-
3 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
2cebc3cca..3a9c9d9a0
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE,
cacheline0,
+dpdk_port_t flow_transfer_proxy_port_id;
This extra field here makes it overflow one cache line.
dpdk_port_t port_id;
/* If true, device was attached by rte_eth_dev_attach(). */
@@ -1183,6
+1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa =
RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
/*
* Full tunnel offload requires that tunnel ID metadata be @@
-1194,6
+1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
*/
dpdk_eth_dev_init_rx_metadata(dev);
+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+ >flow_transfer_proxy_port_id,
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
rte_eth_dev_info_get(dev->port_id, );
if (strstr(info.driver_name, "vf") != NULL) { @@ -3981,8 +4001,10
@@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
const char *argv[], void *aux OVS_UNUSED) {
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn,
int argc OVS_UNUSED,
argv[1]);
RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn
*conn,
int argc OVS_UNUSED,
}
ds_destroy(_interfaces);
+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow
proxy)",
+ argv[1]);
This is not acceptable.
When removing a port, we clean the offloads using
netdev_offload_dpdk_flow_flush().
It should be enhanced to check if the proxy port is detached, remove the
offloads of all the ports that used it.
There is a related patch proposed in [1].
[1]
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
work.ozlabs.org%2Fproject%2Fopenvswitch%2Fpatch%2F20220905144603.358
51
05-1-
elibr%40nvidia.com%2F=05%7C01%7Celibr%40nvidia.com%7Ca5083e7
6e913441830f108db64fb5c71%7C43083d15727340c1b7db39efd9ccc17a%7C0%7
C0%7
Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
>-Original Message-
>From: Ivan Malov
>Sent: Sunday, 4 June 2023 15:58
>To: Eli Britstein
>Cc: ovs-dev@openvswitch.org; Ilya Maximets ; Ori
>Kam ; David Marchand
>Subject: RE: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
>mechanism
>
>External email: Use caution opening links or attachments
>
>
>Hi Eli,
>
>Thanks for reviewing this. Please see below.
>
>On Tue, 21 Feb 2023, Eli Britstein wrote:
>
>>
>>
>>> -Original Message-
>>> From: Ivan Malov
>>> Sent: Tuesday, 21 February 2023 2:41
>>> To: ovs-dev@openvswitch.org
>>> Cc: Ilya Maximets ; Eli Britstein
>>> ; Ori Kam ; David Marchand
>>>
>>> Subject: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
>>> mechanism
>>>
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> Manage "transfer" flows via the corresponding mechanism.
>>> Doing so requires that the traffic source be specified explicitly,
>>> via the corresponding pattern item.
>>>
>>> Signed-off-by: Ivan Malov
>>> ---
>>> lib/netdev-dpdk.c | 88 +++
>>> lib/netdev-dpdk.h | 4 +-
>>> lib/netdev-offload-dpdk.c | 55 +++-
>>> 3 files changed, 117 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>>> 2cebc3cca..3a9c9d9a0
>>> 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
>>>
>>> struct netdev_dpdk {
>>> PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE,
>cacheline0,
>>> +dpdk_port_t flow_transfer_proxy_port_id;
>> This extra field here makes it overflow one cache line.
>>> dpdk_port_t port_id;
>>>
>>> /* If true, device was attached by rte_eth_dev_attach(). */
>>> @@ -1183,6
>>> +1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>>> uint32_t rx_chksm_offload_capa =
>RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
>>> RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
>>> RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
>>> +int ret;
>>>
>>> /*
>>> * Full tunnel offload requires that tunnel ID metadata be @@
>>> -1194,6
>>> +1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
>>> */
>>> dpdk_eth_dev_init_rx_metadata(dev);
>>>
>>> +/*
>>> + * Managing "transfer" flows requires that the user communicate them
>>> + * via a port which has the privilege to control the embedded switch.
>>> + * For some vendors, all ports in a given switching domain have
>>> + * this privilege. For other vendors, it's only one port.
>>> + *
>>> + * Get the proxy port ID and remember it for later use.
>>> + */
>>> +ret = rte_flow_pick_transfer_proxy(dev->port_id,
>>> + >flow_transfer_proxy_port_id,
>>> NULL);
>>> +if (ret != 0) {
>>> +/*
>>> + * The PMD does not indicate the proxy port.
>>> + * Assume the proxy is unneeded.
>>> + */
>>> +dev->flow_transfer_proxy_port_id = dev->port_id;
>>> +}
>>> +
>>> rte_eth_dev_info_get(dev->port_id, );
>>>
>>> if (strstr(info.driver_name, "vf") != NULL) { @@ -3981,8 +4001,10
>>> @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
>OVS_UNUSED,
>>>const char *argv[], void *aux OVS_UNUSED) {
>>> struct ds used_interfaces = DS_EMPTY_INITIALIZER;
>>> +struct netdev_dpdk *dev_self = NULL;
>>> struct rte_eth_dev_info dev_info;
>>> dpdk_port_t sibling_port_id;
>>> +struct netdev_dpdk *dev;
>>> dpdk_port_t port_id;
>>> bool used = false;
>>> char *response;
>>> @@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn,
>>> int argc OVS_UNUSED,
>>> argv[1]);
>>>
>>> RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
>>> -struct netdev_dpdk *dev;
>>> -
>>> LIST_FOR_EACH (dev, list_node, _list) {
>>> if (dev->port_id != sibling_port_id) {
>>> continue;
>>> @@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn
>*conn,
>>> int argc OVS_UNUSED,
>>> }
>>> ds_destroy(_interfaces);
>>>
>>> +/*
>>> + * The device being detached may happen to be a flow proxy port
>>> + * for another device (still attached). If so, do not allow to
>>> + * detach. Devices dependent on this one must be detached first.
>>> + */
>>> +LIST_FOR_EACH (dev, list_node, _list) {
>>> +if (dev->port_id == port_id) {
>>> +dev_self = dev;
>>> +} else if (dev->flow_transfer_proxy_port_id == port_id) {
>>> +response = xasprintf("Device '%s' can not be detached (flow
>proxy)",
>>> + argv[1]);
>> This is not acceptable.
>> When removing a port, we clean the offloads using
>netdev_offload_dpdk_flow_flush().
>> It should be enhanced to check if the proxy port is detached, remove the
>offloads of all the ports that used it.
>> There is a related
Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
Hi Eli,
Thanks for reviewing this. Please see below.
On Tue, 21 Feb 2023, Eli Britstein wrote:
-Original Message-
From: Ivan Malov
Sent: Tuesday, 21 February 2023 2:41
To: ovs-dev@openvswitch.org
Cc: Ilya Maximets ; Eli Britstein ; Ori
Kam ; David Marchand
Subject: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
mechanism
External email: Use caution opening links or attachments
Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified explicitly, via the
corresponding pattern item.
Signed-off-by: Ivan Malov
---
lib/netdev-dpdk.c | 88 +++
lib/netdev-dpdk.h | 4 +-
lib/netdev-offload-dpdk.c | 55 +++-
3 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 2cebc3cca..3a9c9d9a0
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
+dpdk_port_t flow_transfer_proxy_port_id;
This extra field here makes it overflow one cache line.
dpdk_port_t port_id;
/* If true, device was attached by rte_eth_dev_attach(). */ @@ -1183,6
+1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
/*
* Full tunnel offload requires that tunnel ID metadata be @@ -1194,6
+1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
*/
dpdk_eth_dev_init_rx_metadata(dev);
+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+ >flow_transfer_proxy_port_id,
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
rte_eth_dev_info_get(dev->port_id, );
if (strstr(info.driver_name, "vf") != NULL) { @@ -3981,8 +4001,10 @@
netdev_dpdk_detach(struct unixctl_conn *conn, int argc OVS_UNUSED,
const char *argv[], void *aux OVS_UNUSED) {
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int
argc OVS_UNUSED,
argv[1]);
RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn *conn,
int argc OVS_UNUSED,
}
ds_destroy(_interfaces);
+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow
proxy)",
+ argv[1]);
This is not acceptable.
When removing a port, we clean the offloads using
netdev_offload_dpdk_flow_flush().
It should be enhanced to check if the proxy port is detached, remove the
offloads of all the ports that used it.
There is a related patch proposed in [1].
[1]
http://patchwork.ozlabs.org/project/openvswitch/patch/20220905144603.3585105-1-el...@nvidia.com/
I hear you. But do you want me to first wait for patch [1] to be applied
and then send the proxy enhancement to be applied on top of it?
Or do you believe I can do something in parallel here?
As I can see, patch [1] has not been applied yet.
Is there any problem with it?
If I need to wait, then I believe I can revoke this [3/3]
patch from the series for the time being and send v5.
What's your opinion?
+goto error;
+}
+}
+
+/* Indicate that the device being detached no longer needs a flow proxy.
*/
+if (dev_self != NULL)
+dev_self->flow_transfer_proxy_port_id = port_id;
+
Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
>-Original Message-
>From: Ivan Malov
>Sent: Tuesday, 21 February 2023 2:41
>To: ovs-dev@openvswitch.org
>Cc: Ilya Maximets ; Eli Britstein ; Ori
>Kam ; David Marchand
>Subject: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
>mechanism
>
>External email: Use caution opening links or attachments
>
>
>Manage "transfer" flows via the corresponding mechanism.
>Doing so requires that the traffic source be specified explicitly, via the
>corresponding pattern item.
>
>Signed-off-by: Ivan Malov
>---
> lib/netdev-dpdk.c | 88 +++
> lib/netdev-dpdk.h | 4 +-
> lib/netdev-offload-dpdk.c | 55 +++-
> 3 files changed, 117 insertions(+), 30 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 2cebc3cca..3a9c9d9a0
>100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
>
> struct netdev_dpdk {
> PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
>+dpdk_port_t flow_transfer_proxy_port_id;
This extra field here makes it overflow one cache line.
> dpdk_port_t port_id;
>
> /* If true, device was attached by rte_eth_dev_attach(). */ @@ -1183,6
>+1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
> RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
> RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
>+int ret;
>
> /*
> * Full tunnel offload requires that tunnel ID metadata be @@ -1194,6
>+1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> */
> dpdk_eth_dev_init_rx_metadata(dev);
>
>+/*
>+ * Managing "transfer" flows requires that the user communicate them
>+ * via a port which has the privilege to control the embedded switch.
>+ * For some vendors, all ports in a given switching domain have
>+ * this privilege. For other vendors, it's only one port.
>+ *
>+ * Get the proxy port ID and remember it for later use.
>+ */
>+ret = rte_flow_pick_transfer_proxy(dev->port_id,
>+ >flow_transfer_proxy_port_id,
>NULL);
>+if (ret != 0) {
>+/*
>+ * The PMD does not indicate the proxy port.
>+ * Assume the proxy is unneeded.
>+ */
>+dev->flow_transfer_proxy_port_id = dev->port_id;
>+}
>+
> rte_eth_dev_info_get(dev->port_id, );
>
> if (strstr(info.driver_name, "vf") != NULL) { @@ -3981,8 +4001,10 @@
>netdev_dpdk_detach(struct unixctl_conn *conn, int argc OVS_UNUSED,
>const char *argv[], void *aux OVS_UNUSED) {
> struct ds used_interfaces = DS_EMPTY_INITIALIZER;
>+struct netdev_dpdk *dev_self = NULL;
> struct rte_eth_dev_info dev_info;
> dpdk_port_t sibling_port_id;
>+struct netdev_dpdk *dev;
> dpdk_port_t port_id;
> bool used = false;
> char *response;
>@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int
>argc OVS_UNUSED,
> argv[1]);
>
> RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
>-struct netdev_dpdk *dev;
>-
> LIST_FOR_EACH (dev, list_node, _list) {
> if (dev->port_id != sibling_port_id) {
> continue;
>@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn *conn,
>int argc OVS_UNUSED,
> }
> ds_destroy(_interfaces);
>
>+/*
>+ * The device being detached may happen to be a flow proxy port
>+ * for another device (still attached). If so, do not allow to
>+ * detach. Devices dependent on this one must be detached first.
>+ */
>+LIST_FOR_EACH (dev, list_node, _list) {
>+if (dev->port_id == port_id) {
>+dev_self = dev;
>+} else if (dev->flow_transfer_proxy_port_id == port_id) {
>+response = xasprintf("Device '%s' can not be detached (flow
>proxy)",
>+ argv[1]);
This is not acceptable.
When removing a port, we clean the offloads using
netdev_offload_dpdk_flow_flush().
It should be enhanced to check if the proxy port is detached, remove the
offloads of all the ports that used it.
There is a related patch proposed in [1].
[1]
http://patchwork.ozlabs.org/project/openvswitch/patch/20220905144603.3585105-1-el...@nvidia.com/
>+goto error;
>+}
>+}
>+
>+/* Indicate that the device being detached no longer needs a flow proxy.
>*/
>+if (dev_self != NULL)
>+dev_self->flow_transfer_proxy_port_id = port_id;
>+
> rte_eth_dev_info_get(port_id, _info);
> rte_eth_dev_close(port_id);
> if (rte_dev_remove(dev_info.device) < 0) { @@ -5470,7 +5509,8 @@
>unlock:
> }
>
> int
>-netdev_dpdk_get_port_id(struct netdev *netdev)
>+netdev_dpdk_get_port_id(struct netdev *netdev,
>+bool flow_transfer_proxy)
> {
> struct netdev_dpdk *dev;
> int ret = -1;
>@@ -5481,7
Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
Bleep bloop. Greetings Ivan Malov, I am a robot and I have tried out your
patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 80 characters long (recommended limit is 79)
#50 FILE: lib/netdev-dpdk.c:1208:
>flow_transfer_proxy_port_id, NULL);
WARNING: Line is 80 characters long (recommended limit is 79)
#95 FILE: lib/netdev-dpdk.c:4053:
response = xasprintf("Device '%s' can not be detached (flow proxy)",
ERROR: Inappropriate bracing around statement
#102 FILE: lib/netdev-dpdk.c:4060:
if (dev_self != NULL)
WARNING: Line is 80 characters long (recommended limit is 79)
#123 FILE: lib/netdev-dpdk.c:5524:
ret = flow_transfer_proxy ? dev->flow_transfer_proxy_port_id : dev->port_id;
WARNING: Line is 80 characters long (recommended limit is 79)
#212 FILE: lib/netdev-dpdk.c:5709:
ret = rte_flow_tunnel_action_decap_release(dev->flow_transfer_proxy_port_id,
Lines checked: 415, Warnings: 4, Errors: 1
Please check this out. If you feel there has been an error, please email
acon...@redhat.com
Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
Bleep bloop. Greetings Ivan Malov, I am a robot and I have tried out your
patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 80 characters long (recommended limit is 79)
#50 FILE: lib/netdev-dpdk.c:1208:
>flow_transfer_proxy_port_id, NULL);
WARNING: Line is 80 characters long (recommended limit is 79)
#95 FILE: lib/netdev-dpdk.c:4053:
response = xasprintf("Device '%s' can not be detached (flow proxy)",
ERROR: Inappropriate bracing around statement
#102 FILE: lib/netdev-dpdk.c:4060:
if (dev_self != NULL)
WARNING: Line is 80 characters long (recommended limit is 79)
#123 FILE: lib/netdev-dpdk.c:5524:
ret = flow_transfer_proxy ? dev->flow_transfer_proxy_port_id : dev->port_id;
WARNING: Line is 80 characters long (recommended limit is 79)
#212 FILE: lib/netdev-dpdk.c:5709:
ret = rte_flow_tunnel_action_decap_release(dev->flow_transfer_proxy_port_id,
Lines checked: 415, Warnings: 4, Errors: 1
Please check this out. If you feel there has been an error, please email
acon...@redhat.com
Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified
explicitly, via the corresponding pattern item.
Signed-off-by: Ivan Malov
---
lib/netdev-dpdk.c | 88 +++
lib/netdev-dpdk.h | 4 +-
lib/netdev-offload-dpdk.c | 55 +++-
3 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2cebc3cca..3a9c9d9a0 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
+dpdk_port_t flow_transfer_proxy_port_id;
dpdk_port_t port_id;
/* If true, device was attached by rte_eth_dev_attach(). */
@@ -1183,6 +1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
/*
* Full tunnel offload requires that tunnel ID metadata be
@@ -1194,6 +1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
*/
dpdk_eth_dev_init_rx_metadata(dev);
+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+ >flow_transfer_proxy_port_id,
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
rte_eth_dev_info_get(dev->port_id, );
if (strstr(info.driver_name, "vf") != NULL) {
@@ -3981,8 +4001,10 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
const char *argv[], void *aux OVS_UNUSED)
{
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
argv[1]);
RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
}
ds_destroy(_interfaces);
+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow
proxy)",
+ argv[1]);
+goto error;
+}
+}
+
+/* Indicate that the device being detached no longer needs a flow proxy. */
+if (dev_self != NULL)
+dev_self->flow_transfer_proxy_port_id = port_id;
+
rte_eth_dev_info_get(port_id, _info);
rte_eth_dev_close(port_id);
if (rte_dev_remove(dev_info.device) < 0) {
@@ -5470,7 +5509,8 @@ unlock:
}
int
-netdev_dpdk_get_port_id(struct netdev *netdev)
+netdev_dpdk_get_port_id(struct netdev *netdev,
+bool flow_transfer_proxy)
{
struct netdev_dpdk *dev;
int ret = -1;
@@ -5481,7 +5521,7 @@ netdev_dpdk_get_port_id(struct netdev *netdev)
dev = netdev_dpdk_cast(netdev);
ovs_mutex_lock(>mutex);
-ret = dev->port_id;
+ret = flow_transfer_proxy ? dev->flow_transfer_proxy_port_id :
dev->port_id;
ovs_mutex_unlock(>mutex);
out:
return ret;
@@ -5517,13 +5557,15 @@ out:
int
netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
- struct rte_flow *rte_flow,
+ bool transfer, struct rte_flow *rte_flow,
struct rte_flow_error *error)
{
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
int ret;
-ret = rte_flow_destroy(dev->port_id, rte_flow, error);
+ret = rte_flow_destroy(transfer ?
+ dev->flow_transfer_proxy_port_id : dev->port_id,
+ rte_flow,
[ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism
Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified
explicitly, via the corresponding pattern item.
Signed-off-by: Ivan Malov
---
lib/netdev-dpdk.c | 88 +++
lib/netdev-dpdk.h | 4 +-
lib/netdev-offload-dpdk.c | 55 +++-
3 files changed, 117 insertions(+), 30 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2cebc3cca..3a9c9d9a0 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
+dpdk_port_t flow_transfer_proxy_port_id;
dpdk_port_t port_id;
/* If true, device was attached by rte_eth_dev_attach(). */
@@ -1183,6 +1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
/*
* Full tunnel offload requires that tunnel ID metadata be
@@ -1194,6 +1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
*/
dpdk_eth_dev_init_rx_metadata(dev);
+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+ >flow_transfer_proxy_port_id,
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
rte_eth_dev_info_get(dev->port_id, );
if (strstr(info.driver_name, "vf") != NULL) {
@@ -3981,8 +4001,10 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
const char *argv[], void *aux OVS_UNUSED)
{
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
argv[1]);
RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc
OVS_UNUSED,
}
ds_destroy(_interfaces);
+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow
proxy)",
+ argv[1]);
+goto error;
+}
+}
+
+/* Indicate that the device being detached no longer needs a flow proxy. */
+if (dev_self != NULL)
+dev_self->flow_transfer_proxy_port_id = port_id;
+
rte_eth_dev_info_get(port_id, _info);
rte_eth_dev_close(port_id);
if (rte_dev_remove(dev_info.device) < 0) {
@@ -5470,7 +5509,8 @@ unlock:
}
int
-netdev_dpdk_get_port_id(struct netdev *netdev)
+netdev_dpdk_get_port_id(struct netdev *netdev,
+bool flow_transfer_proxy)
{
struct netdev_dpdk *dev;
int ret = -1;
@@ -5481,7 +5521,7 @@ netdev_dpdk_get_port_id(struct netdev *netdev)
dev = netdev_dpdk_cast(netdev);
ovs_mutex_lock(>mutex);
-ret = dev->port_id;
+ret = flow_transfer_proxy ? dev->flow_transfer_proxy_port_id :
dev->port_id;
ovs_mutex_unlock(>mutex);
out:
return ret;
@@ -5517,13 +5557,15 @@ out:
int
netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
- struct rte_flow *rte_flow,
+ bool transfer, struct rte_flow *rte_flow,
struct rte_flow_error *error)
{
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
int ret;
-ret = rte_flow_destroy(dev->port_id, rte_flow, error);
+ret = rte_flow_destroy(transfer ?
+ dev->flow_transfer_proxy_port_id : dev->port_id,
+ rte_flow,
