Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
On 2022-03-18 7:55 PM, Eelco Chaudron wrote:
On 17 Mar 2022, at 2:01, Chris Mi wrote:
On 2022-03-11 8:53 PM, Eelco Chaudron wrote:
@@ -449,6 +462,7 @@ dpif_close(struct dpif *dpif)
if (dpif) {
struct registered_dpif_class *rc;
+dpif_offload_close(dpif);
** Not sure I understand, but why are we destroying the offload dpif class
here, it can be used by another dpif type.
** I guess this is all because your design has a 1:1 mapping? Guess it should
be two dpif_types that could share the same offload class type.
Now it is moved to dpif_netlink_close().
Except the 1:1 mapping comment which I think need Ilya's feedback, I have
addressed your other comments.
Thanks for your comments. The dpif-offload for dummy is not needed and removed.
If needed, I can send v21.
Thanks for taking care of the questions and fixing them in your sandbox.
I would prefer for you to not send any more revisions until we have a clear
answer from Ilya.
Since Ilya didn't reply, I'll send a new version to reflect the latest change.
Well, my goal was to not do any more reviews until Ilya would reply, as every
review cycle takes up quite some time.
OK, hopefully Ilya could review it soon.
However I could not get v21 to apply cleanly, so I will hold off on any further
reviews of this series until we have a clear direction from Ilya.
The reason is because we thought patch "tc: Keep header rewrite actions
order" should be merged first.
So this series is rebased on that.
-Chris
//Eelco
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
On 17 Mar 2022, at 2:01, Chris Mi wrote:
> On 2022-03-11 8:53 PM, Eelco Chaudron wrote:
>>
>
> @@ -449,6 +462,7 @@ dpif_close(struct dpif *dpif)
>if (dpif) {
>struct registered_dpif_class *rc;
>
> +dpif_offload_close(dpif);
** Not sure I understand, but why are we destroying the offload dpif class
here, it can be used by another dpif type.
** I guess this is all because your design has a 1:1 mapping? Guess it
should be two dpif_types that could share the same offload class type.
>>> Now it is moved to dpif_netlink_close().
>>>
>>> Except the 1:1 mapping comment which I think need Ilya's feedback, I have
>>> addressed your other comments.
>>> Thanks for your comments. The dpif-offload for dummy is not needed and
>>> removed.
>>> If needed, I can send v21.
>>
>> Thanks for taking care of the questions and fixing them in your sandbox.
>> I would prefer for you to not send any more revisions until we have a clear
>> answer from Ilya.
> Since Ilya didn't reply, I'll send a new version to reflect the latest change.
Well, my goal was to not do any more reviews until Ilya would reply, as every
review cycle takes up quite some time.
However I could not get v21 to apply cleanly, so I will hold off on any further
reviews of this series until we have a clear direction from Ilya.
//Eelco
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
On 2022-03-11 8:53 PM, Eelco Chaudron wrote:
On 11 Mar 2022, at 3:14, Chris Mi wrote:
On 2022-03-04 6:50 PM, Eelco Chaudron wrote:
I’m re-adding some of my v18 comments, which I know are depending on Ilya’s
feedback, although I still think we should have provider classes, i.e., not
based on the datapath class.
Some other questions could still be answered regardless of Ilya’s feedback
(will mark them with **). Please answer them inline, before sending another
revision so we can have some discussion around them if needed.
OK.
//Eelco
On 15 Feb 2022, at 10:17, Chris Mi wrote:
Implement dpif-offload API for netlink datapath. And implement a
dummy dpif-offload API for netdev datapath to make tests pass.
** Why do we need a dummy? If there is no hardware offload class we should just
pass, shouldn’t we?
According to one of your below comment, I moved dp_offload_class_lookup from
dpif to dpif-netlink.
After that, dpif-offload for dummy is not needed.
Issue: 2181036
Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
Signed-off-by: Chris Mi
Reviewed-by: Eli Britstein
---
lib/automake.mk | 2 +
lib/dpif-netdev.c | 8 +-
lib/dpif-netlink.c | 4 +-
lib/dpif-offload-netdev.c | 43 +++
lib/dpif-offload-netlink.c | 221
lib/dpif-offload-provider.h | 25 +++-
lib/dpif-offload.c | 157 +
lib/dpif-provider.h | 6 +-
lib/dpif.c | 17 ++-
9 files changed, 476 insertions(+), 7 deletions(-)
create mode 100644 lib/dpif-offload-netdev.c
create mode 100644 lib/dpif-offload-netlink.c
diff --git a/lib/automake.mk b/lib/automake.mk
index 781fba47a..9ed61029a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpif-netdev-perf.c \
lib/dpif-netdev-perf.h \
lib/dpif-offload.c \
+ lib/dpif-offload-netdev.c \
lib/dpif-offload-provider.h \
lib/dpif-provider.h \
lib/dpif.c \
@@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.h \
lib/dpif-netlink-rtnl.c \
lib/dpif-netlink-rtnl.h \
+ lib/dpif-offload-netlink.c \
lib/if-notifier.c \
lib/netdev-linux.c \
lib/netdev-linux.h \
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e28e0b554..5ebd127b9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
ovs_refcount_ref(>ref_cnt);
dpif = xmalloc(sizeof *dpif);
-dpif_init(>dpif, dp->class, dp->name, netflow_id >> 8, netflow_id);
+dpif_init(>dpif, dp->class, NULL, dp->name, netflow_id >> 8,
+ netflow_id);
dpif->dp = dp;
dpif->last_port_seq = seq_read(dp->port_seq);
@@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
if (error == 0 || error == EAFNOSUPPORT) {
dpif_dummy_register__(type);
}
+error = dp_offload_unregister_provider(type);
+if (error == 0 || error == EAFNOSUPPORT) {
+dpif_offload_dummy_register(type);
+}
}
void
@@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
}
dpif_dummy_register__("dummy");
+dpif_offload_dummy_register("dummy");
unixctl_command_register("dpif-dummy/change-port-number",
"dp port new-number",
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 71e35ccdd..75f85c254 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif
**dpifp)
dpif->port_notifier = NULL;
fat_rwlock_init(>upcall_lock);
-dpif_init(>dpif, _netlink_class, dp->name,
- dp->dp_ifindex, dp->dp_ifindex);
+dpif_init(>dpif, _netlink_class, _offload_netlink_class,
+ dp->name, dp->dp_ifindex, dp->dp_ifindex);
dpif->dp_ifindex = dp->dp_ifindex;
dpif->user_features = dp->user_features;
diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
new file mode 100644
index 0..35dac2cc3
--- /dev/null
+++ b/lib/dpif-offload-netdev.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.apache.org%2Flicenses%2FLICENSE-2.0data=04%7C01%7Ccmi%40nvidia.com%7Caea6ff519e9c41a4a21108da035e3272%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637826000390994283%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=QzFQcAWcmd9qjB6dvsLOmULRkaHqYD1n1kOifjqRZgo%3Dreserved=0
+ *
+ * Unless required by applicable law or agreed to in writing,
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
On 2022-03-11 8:53 PM, Eelco Chaudron wrote:
On 11 Mar 2022, at 3:14, Chris Mi wrote:
On 2022-03-04 6:50 PM, Eelco Chaudron wrote:
I’m re-adding some of my v18 comments, which I know are depending on Ilya’s
feedback, although I still think we should have provider classes, i.e., not
based on the datapath class.
Some other questions could still be answered regardless of Ilya’s feedback
(will mark them with **). Please answer them inline, before sending another
revision so we can have some discussion around them if needed.
OK.
//Eelco
On 15 Feb 2022, at 10:17, Chris Mi wrote:
Implement dpif-offload API for netlink datapath. And implement a
dummy dpif-offload API for netdev datapath to make tests pass.
** Why do we need a dummy? If there is no hardware offload class we should just
pass, shouldn’t we?
According to one of your below comment, I moved dp_offload_class_lookup from
dpif to dpif-netlink.
After that, dpif-offload for dummy is not needed.
Issue: 2181036
Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
Signed-off-by: Chris Mi
Reviewed-by: Eli Britstein
---
lib/automake.mk | 2 +
lib/dpif-netdev.c | 8 +-
lib/dpif-netlink.c | 4 +-
lib/dpif-offload-netdev.c | 43 +++
lib/dpif-offload-netlink.c | 221
lib/dpif-offload-provider.h | 25 +++-
lib/dpif-offload.c | 157 +
lib/dpif-provider.h | 6 +-
lib/dpif.c | 17 ++-
9 files changed, 476 insertions(+), 7 deletions(-)
create mode 100644 lib/dpif-offload-netdev.c
create mode 100644 lib/dpif-offload-netlink.c
diff --git a/lib/automake.mk b/lib/automake.mk
index 781fba47a..9ed61029a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpif-netdev-perf.c \
lib/dpif-netdev-perf.h \
lib/dpif-offload.c \
+ lib/dpif-offload-netdev.c \
lib/dpif-offload-provider.h \
lib/dpif-provider.h \
lib/dpif.c \
@@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.h \
lib/dpif-netlink-rtnl.c \
lib/dpif-netlink-rtnl.h \
+ lib/dpif-offload-netlink.c \
lib/if-notifier.c \
lib/netdev-linux.c \
lib/netdev-linux.h \
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e28e0b554..5ebd127b9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
ovs_refcount_ref(>ref_cnt);
dpif = xmalloc(sizeof *dpif);
-dpif_init(>dpif, dp->class, dp->name, netflow_id >> 8, netflow_id);
+dpif_init(>dpif, dp->class, NULL, dp->name, netflow_id >> 8,
+ netflow_id);
dpif->dp = dp;
dpif->last_port_seq = seq_read(dp->port_seq);
@@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
if (error == 0 || error == EAFNOSUPPORT) {
dpif_dummy_register__(type);
}
+error = dp_offload_unregister_provider(type);
+if (error == 0 || error == EAFNOSUPPORT) {
+dpif_offload_dummy_register(type);
+}
}
void
@@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
}
dpif_dummy_register__("dummy");
+dpif_offload_dummy_register("dummy");
unixctl_command_register("dpif-dummy/change-port-number",
"dp port new-number",
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 71e35ccdd..75f85c254 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif
**dpifp)
dpif->port_notifier = NULL;
fat_rwlock_init(>upcall_lock);
-dpif_init(>dpif, _netlink_class, dp->name,
- dp->dp_ifindex, dp->dp_ifindex);
+dpif_init(>dpif, _netlink_class, _offload_netlink_class,
+ dp->name, dp->dp_ifindex, dp->dp_ifindex);
dpif->dp_ifindex = dp->dp_ifindex;
dpif->user_features = dp->user_features;
diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
new file mode 100644
index 0..35dac2cc3
--- /dev/null
+++ b/lib/dpif-offload-netdev.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.apache.org%2Flicenses%2FLICENSE-2.0data=04%7C01%7Ccmi%40nvidia.com%7Caea6ff519e9c41a4a21108da035e3272%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637826000390994283%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=QzFQcAWcmd9qjB6dvsLOmULRkaHqYD1n1kOifjqRZgo%3Dreserved=0
+ *
+ * Unless required by applicable law or agreed to in writing,
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
On 11 Mar 2022, at 3:14, Chris Mi wrote:
> On 2022-03-04 6:50 PM, Eelco Chaudron wrote:
>> I’m re-adding some of my v18 comments, which I know are depending on Ilya’s
>> feedback, although I still think we should have provider classes, i.e., not
>> based on the datapath class.
>>
>> Some other questions could still be answered regardless of Ilya’s feedback
>> (will mark them with **). Please answer them inline, before sending another
>> revision so we can have some discussion around them if needed.
> OK.
>>
>> //Eelco
>>
>> On 15 Feb 2022, at 10:17, Chris Mi wrote:
>>
>>> Implement dpif-offload API for netlink datapath. And implement a
>>> dummy dpif-offload API for netdev datapath to make tests pass.
>>
>> ** Why do we need a dummy? If there is no hardware offload class we should
>> just pass, shouldn’t we?
> According to one of your below comment, I moved dp_offload_class_lookup from
> dpif to dpif-netlink.
> After that, dpif-offload for dummy is not needed.
>>
>>> Issue: 2181036
>>> Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
>>> Signed-off-by: Chris Mi
>>> Reviewed-by: Eli Britstein
>>> ---
>>> lib/automake.mk | 2 +
>>> lib/dpif-netdev.c | 8 +-
>>> lib/dpif-netlink.c | 4 +-
>>> lib/dpif-offload-netdev.c | 43 +++
>>> lib/dpif-offload-netlink.c | 221
>>> lib/dpif-offload-provider.h | 25 +++-
>>> lib/dpif-offload.c | 157 +
>>> lib/dpif-provider.h | 6 +-
>>> lib/dpif.c | 17 ++-
>>> 9 files changed, 476 insertions(+), 7 deletions(-)
>>> create mode 100644 lib/dpif-offload-netdev.c
>>> create mode 100644 lib/dpif-offload-netlink.c
>>>
>>> diff --git a/lib/automake.mk b/lib/automake.mk
>>> index 781fba47a..9ed61029a 100644
>>> --- a/lib/automake.mk
>>> +++ b/lib/automake.mk
>>> @@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
>>> lib/dpif-netdev-perf.c \
>>> lib/dpif-netdev-perf.h \
>>> lib/dpif-offload.c \
>>> + lib/dpif-offload-netdev.c \
>>> lib/dpif-offload-provider.h \
>>> lib/dpif-provider.h \
>>> lib/dpif.c \
>>> @@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
>>> lib/dpif-netlink.h \
>>> lib/dpif-netlink-rtnl.c \
>>> lib/dpif-netlink-rtnl.h \
>>> + lib/dpif-offload-netlink.c \
>>> lib/if-notifier.c \
>>> lib/netdev-linux.c \
>>> lib/netdev-linux.h \
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index e28e0b554..5ebd127b9 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
>>> ovs_refcount_ref(>ref_cnt);
>>>
>>> dpif = xmalloc(sizeof *dpif);
>>> -dpif_init(>dpif, dp->class, dp->name, netflow_id >> 8,
>>> netflow_id);
>>> +dpif_init(>dpif, dp->class, NULL, dp->name, netflow_id >> 8,
>>> + netflow_id);
>>> dpif->dp = dp;
>>> dpif->last_port_seq = seq_read(dp->port_seq);
>>>
>>> @@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
>>> if (error == 0 || error == EAFNOSUPPORT) {
>>> dpif_dummy_register__(type);
>>> }
>>> +error = dp_offload_unregister_provider(type);
>>> +if (error == 0 || error == EAFNOSUPPORT) {
>>> +dpif_offload_dummy_register(type);
>>> +}
>>> }
>>>
>>> void
>>> @@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
>>> }
>>>
>>> dpif_dummy_register__("dummy");
>>> +dpif_offload_dummy_register("dummy");
>>>
>>> unixctl_command_register("dpif-dummy/change-port-number",
>>>"dp port new-number",
>>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>>> index 71e35ccdd..75f85c254 100644
>>> --- a/lib/dpif-netlink.c
>>> +++ b/lib/dpif-netlink.c
>>> @@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif
>>> **dpifp)
>>> dpif->port_notifier = NULL;
>>> fat_rwlock_init(>upcall_lock);
>>>
>>> -dpif_init(>dpif, _netlink_class, dp->name,
>>> - dp->dp_ifindex, dp->dp_ifindex);
>>> +dpif_init(>dpif, _netlink_class,
>>> _offload_netlink_class,
>>> + dp->name, dp->dp_ifindex, dp->dp_ifindex);
>>>
>>> dpif->dp_ifindex = dp->dp_ifindex;
>>> dpif->user_features = dp->user_features;
>>> diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
>>> new file mode 100644
>>> index 0..35dac2cc3
>>> --- /dev/null
>>> +++ b/lib/dpif-offload-netdev.c
>>> @@ -0,0 +1,43 @@
>>> +/*
>>> + * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights
>>> reserved.
>>> + *
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at:
>>> + *
>>> + *
>>>
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
On 2022-03-04 6:50 PM, Eelco Chaudron wrote:
I’m re-adding some of my v18 comments, which I know are depending on Ilya’s
feedback, although I still think we should have provider classes, i.e., not
based on the datapath class.
Some other questions could still be answered regardless of Ilya’s feedback
(will mark them with **). Please answer them inline, before sending another
revision so we can have some discussion around them if needed.
OK.
//Eelco
On 15 Feb 2022, at 10:17, Chris Mi wrote:
Implement dpif-offload API for netlink datapath. And implement a
dummy dpif-offload API for netdev datapath to make tests pass.
** Why do we need a dummy? If there is no hardware offload class we should just
pass, shouldn’t we?
According to one of your below comment, I moved dp_offload_class_lookup
from dpif to dpif-netlink.
After that, dpif-offload for dummy is not needed.
Issue: 2181036
Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
Signed-off-by: Chris Mi
Reviewed-by: Eli Britstein
---
lib/automake.mk | 2 +
lib/dpif-netdev.c | 8 +-
lib/dpif-netlink.c | 4 +-
lib/dpif-offload-netdev.c | 43 +++
lib/dpif-offload-netlink.c | 221
lib/dpif-offload-provider.h | 25 +++-
lib/dpif-offload.c | 157 +
lib/dpif-provider.h | 6 +-
lib/dpif.c | 17 ++-
9 files changed, 476 insertions(+), 7 deletions(-)
create mode 100644 lib/dpif-offload-netdev.c
create mode 100644 lib/dpif-offload-netlink.c
diff --git a/lib/automake.mk b/lib/automake.mk
index 781fba47a..9ed61029a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpif-netdev-perf.c \
lib/dpif-netdev-perf.h \
lib/dpif-offload.c \
+ lib/dpif-offload-netdev.c \
lib/dpif-offload-provider.h \
lib/dpif-provider.h \
lib/dpif.c \
@@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.h \
lib/dpif-netlink-rtnl.c \
lib/dpif-netlink-rtnl.h \
+ lib/dpif-offload-netlink.c \
lib/if-notifier.c \
lib/netdev-linux.c \
lib/netdev-linux.h \
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e28e0b554..5ebd127b9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
ovs_refcount_ref(>ref_cnt);
dpif = xmalloc(sizeof *dpif);
-dpif_init(>dpif, dp->class, dp->name, netflow_id >> 8, netflow_id);
+dpif_init(>dpif, dp->class, NULL, dp->name, netflow_id >> 8,
+ netflow_id);
dpif->dp = dp;
dpif->last_port_seq = seq_read(dp->port_seq);
@@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
if (error == 0 || error == EAFNOSUPPORT) {
dpif_dummy_register__(type);
}
+error = dp_offload_unregister_provider(type);
+if (error == 0 || error == EAFNOSUPPORT) {
+dpif_offload_dummy_register(type);
+}
}
void
@@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
}
dpif_dummy_register__("dummy");
+dpif_offload_dummy_register("dummy");
unixctl_command_register("dpif-dummy/change-port-number",
"dp port new-number",
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 71e35ccdd..75f85c254 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif
**dpifp)
dpif->port_notifier = NULL;
fat_rwlock_init(>upcall_lock);
-dpif_init(>dpif, _netlink_class, dp->name,
- dp->dp_ifindex, dp->dp_ifindex);
+dpif_init(>dpif, _netlink_class, _offload_netlink_class,
+ dp->name, dp->dp_ifindex, dp->dp_ifindex);
dpif->dp_ifindex = dp->dp_ifindex;
dpif->user_features = dp->user_features;
diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
new file mode 100644
index 0..35dac2cc3
--- /dev/null
+++ b/lib/dpif-offload-netdev.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.apache.org%2Flicenses%2FLICENSE-2.0data=04%7C01%7Ccmi%40nvidia.com%7Cadd31a18c66c46ea056408d9fdccd97d%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637819878551136144%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=A5fNu%2BjqL48flvxwkfz1BHOAiu6LLuBTdp%2F4HkYVEdw%3Dreserved=0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
I’m re-adding some of my v18 comments, which I know are depending on Ilya’s
feedback, although I still think we should have provider classes, i.e., not
based on the datapath class.
Some other questions could still be answered regardless of Ilya’s feedback
(will mark them with **). Please answer them inline, before sending another
revision so we can have some discussion around them if needed.
//Eelco
On 15 Feb 2022, at 10:17, Chris Mi wrote:
> Implement dpif-offload API for netlink datapath. And implement a
> dummy dpif-offload API for netdev datapath to make tests pass.
** Why do we need a dummy? If there is no hardware offload class we should just
pass, shouldn’t we?
>
> Issue: 2181036
> Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
> Signed-off-by: Chris Mi
> Reviewed-by: Eli Britstein
> ---
> lib/automake.mk | 2 +
> lib/dpif-netdev.c | 8 +-
> lib/dpif-netlink.c | 4 +-
> lib/dpif-offload-netdev.c | 43 +++
> lib/dpif-offload-netlink.c | 221
> lib/dpif-offload-provider.h | 25 +++-
> lib/dpif-offload.c | 157 +
> lib/dpif-provider.h | 6 +-
> lib/dpif.c | 17 ++-
> 9 files changed, 476 insertions(+), 7 deletions(-)
> create mode 100644 lib/dpif-offload-netdev.c
> create mode 100644 lib/dpif-offload-netlink.c
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 781fba47a..9ed61029a 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
> lib/dpif-netdev-perf.c \
> lib/dpif-netdev-perf.h \
> lib/dpif-offload.c \
> + lib/dpif-offload-netdev.c \
> lib/dpif-offload-provider.h \
> lib/dpif-provider.h \
> lib/dpif.c \
> @@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
> lib/dpif-netlink.h \
> lib/dpif-netlink-rtnl.c \
> lib/dpif-netlink-rtnl.h \
> + lib/dpif-offload-netlink.c \
> lib/if-notifier.c \
> lib/netdev-linux.c \
> lib/netdev-linux.h \
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index e28e0b554..5ebd127b9 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
> ovs_refcount_ref(>ref_cnt);
>
> dpif = xmalloc(sizeof *dpif);
> -dpif_init(>dpif, dp->class, dp->name, netflow_id >> 8, netflow_id);
> +dpif_init(>dpif, dp->class, NULL, dp->name, netflow_id >> 8,
> + netflow_id);
> dpif->dp = dp;
> dpif->last_port_seq = seq_read(dp->port_seq);
>
> @@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
> if (error == 0 || error == EAFNOSUPPORT) {
> dpif_dummy_register__(type);
> }
> +error = dp_offload_unregister_provider(type);
> +if (error == 0 || error == EAFNOSUPPORT) {
> +dpif_offload_dummy_register(type);
> +}
> }
>
> void
> @@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
> }
>
> dpif_dummy_register__("dummy");
> +dpif_offload_dummy_register("dummy");
>
> unixctl_command_register("dpif-dummy/change-port-number",
> "dp port new-number",
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 71e35ccdd..75f85c254 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif
> **dpifp)
> dpif->port_notifier = NULL;
> fat_rwlock_init(>upcall_lock);
>
> -dpif_init(>dpif, _netlink_class, dp->name,
> - dp->dp_ifindex, dp->dp_ifindex);
> +dpif_init(>dpif, _netlink_class, _offload_netlink_class,
> + dp->name, dp->dp_ifindex, dp->dp_ifindex);
>
> dpif->dp_ifindex = dp->dp_ifindex;
> dpif->user_features = dp->user_features;
> diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
> new file mode 100644
> index 0..35dac2cc3
> --- /dev/null
> +++ b/lib/dpif-offload-netdev.c
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include
> +
> +#include "dpif.h"
> +#include "dpif-offload-provider.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(dpif_offload_netdev);
> +
> +/* Currently, it is used only for dummy netdev to make tests pass. */
> +const
Re: [ovs-dev] [PATCH v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
Bleep bloop. Greetings Chris Mi, 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: ERROR: Remove Gerrit Change-Id's before submitting upstream. 9: Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a Lines checked: 673, Warnings: 0, 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 v20 5/8] dpif-offload-netlink: Implement dpif-offload-provider API
Implement dpif-offload API for netlink datapath. And implement a
dummy dpif-offload API for netdev datapath to make tests pass.
Issue: 2181036
Change-Id: I950c3cc3c7092c3d87602da8928ad4aa2df4f38a
Signed-off-by: Chris Mi
Reviewed-by: Eli Britstein
---
lib/automake.mk | 2 +
lib/dpif-netdev.c | 8 +-
lib/dpif-netlink.c | 4 +-
lib/dpif-offload-netdev.c | 43 +++
lib/dpif-offload-netlink.c | 221
lib/dpif-offload-provider.h | 25 +++-
lib/dpif-offload.c | 157 +
lib/dpif-provider.h | 6 +-
lib/dpif.c | 17 ++-
9 files changed, 476 insertions(+), 7 deletions(-)
create mode 100644 lib/dpif-offload-netdev.c
create mode 100644 lib/dpif-offload-netlink.c
diff --git a/lib/automake.mk b/lib/automake.mk
index 781fba47a..9ed61029a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -130,6 +130,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpif-netdev-perf.c \
lib/dpif-netdev-perf.h \
lib/dpif-offload.c \
+ lib/dpif-offload-netdev.c \
lib/dpif-offload-provider.h \
lib/dpif-provider.h \
lib/dpif.c \
@@ -453,6 +454,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.h \
lib/dpif-netlink-rtnl.c \
lib/dpif-netlink-rtnl.h \
+ lib/dpif-offload-netlink.c \
lib/if-notifier.c \
lib/netdev-linux.c \
lib/netdev-linux.h \
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index e28e0b554..5ebd127b9 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1683,7 +1683,8 @@ create_dpif_netdev(struct dp_netdev *dp)
ovs_refcount_ref(>ref_cnt);
dpif = xmalloc(sizeof *dpif);
-dpif_init(>dpif, dp->class, dp->name, netflow_id >> 8, netflow_id);
+dpif_init(>dpif, dp->class, NULL, dp->name, netflow_id >> 8,
+ netflow_id);
dpif->dp = dp;
dpif->last_port_seq = seq_read(dp->port_seq);
@@ -9639,6 +9640,10 @@ dpif_dummy_override(const char *type)
if (error == 0 || error == EAFNOSUPPORT) {
dpif_dummy_register__(type);
}
+error = dp_offload_unregister_provider(type);
+if (error == 0 || error == EAFNOSUPPORT) {
+dpif_offload_dummy_register(type);
+}
}
void
@@ -9659,6 +9664,7 @@ dpif_dummy_register(enum dummy_level level)
}
dpif_dummy_register__("dummy");
+dpif_offload_dummy_register("dummy");
unixctl_command_register("dpif-dummy/change-port-number",
"dp port new-number",
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 71e35ccdd..75f85c254 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -461,8 +461,8 @@ open_dpif(const struct dpif_netlink_dp *dp, struct dpif
**dpifp)
dpif->port_notifier = NULL;
fat_rwlock_init(>upcall_lock);
-dpif_init(>dpif, _netlink_class, dp->name,
- dp->dp_ifindex, dp->dp_ifindex);
+dpif_init(>dpif, _netlink_class, _offload_netlink_class,
+ dp->name, dp->dp_ifindex, dp->dp_ifindex);
dpif->dp_ifindex = dp->dp_ifindex;
dpif->user_features = dp->user_features;
diff --git a/lib/dpif-offload-netdev.c b/lib/dpif-offload-netdev.c
new file mode 100644
index 0..35dac2cc3
--- /dev/null
+++ b/lib/dpif-offload-netdev.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "dpif.h"
+#include "dpif-offload-provider.h"
+#include "openvswitch/vlog.h"
+
+VLOG_DEFINE_THIS_MODULE(dpif_offload_netdev);
+
+/* Currently, it is used only for dummy netdev to make tests pass. */
+const struct dpif_offload_class dpif_offload_netdev_class = {
+.type = "netdev",
+.init = NULL,
+.destroy = NULL,
+.sflow_recv_wait = NULL,
+.sflow_recv = NULL,
+};
+
+void
+dpif_offload_dummy_register(const char *type)
+{
+struct dpif_offload_class *class;
+
+class = xmalloc(sizeof *class);
+*class = dpif_offload_netdev_class;
+class->type = xstrdup(type);
+dp_offload_register_provider(class);
+}
diff --git a/lib/dpif-offload-netlink.c b/lib/dpif-offload-netlink.c
new file mode 100644
index 0..02aea7e2d
--- /dev/null
+++ b/lib/dpif-offload-netlink.c
@@ -0,0 +1,221 @@
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the
