Re: [ovs-dev] [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new file
Hi Ilya, all styling issues of this commit were addressed per your review.
They will be updated in v3
> -Original Message-
> From: Ilya Maximets
> Sent: Tuesday, February 26, 2019 4:38 PM
> To: Roni Bar Yanai ; Ophir Munk
> ; ovs-dev@openvswitch.org
> Cc: Ian Stokes ; Olga Shern ;
> Kevin Traynor ; Asaf Penso ;
> Flavio Leitner
> Subject: Re: [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new
> file
>
> On 21.02.2019 19:31, Roni Bar Yanai wrote:
> >
> >
> >> -Original Message-
> >> From: Ilya Maximets
> >> Sent: Thursday, February 21, 2019 6:20 PM
> >> To: Ophir Munk ; ovs-dev@openvswitch.org
> >> Cc: Ian Stokes ; Olga Shern
> ;
> >> Kevin Traynor ; Asaf Penso
> ;
> >> Roni Bar Yanai ; Flavio Leitner
>
> >> Subject: Re: [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new
> >> file
> >>
> >> At first, 'git am' fails to apply the patch:
> >>
> >> Applying: netdev-dpdk: Move offloading-code to a new file
> >> error: patch failed: lib/netdev-dpdk.c:4240
> >> error: lib/netdev-dpdk.c: patch does not apply
> >> .git/rebase-apply/patch:1564: new blank line at EOF.
> >> +
> >> Patch failed at 0002 netdev-dpdk: Move offloading-code to a new file
> >>
> >> I'm not sure why it complains about 'lib/netdev-dpdk.c' because the
> >> issue is the blanlk line at the end of netdev-rte-offloads.c.
Hopefully it is fixed in v3.
> >>
> >> On 21.02.2019 15:07, Ophir Munk wrote:
> >>> From: Roni Bar Yanai
> >>>
> >>> Hardware offloading-code is moved to a new file called
> >>
> >> Just curious, why you writing "offloading-code" as a single dash
> >> separated word?
> >>
> >>> netdev-rte-offloads.c. The original offloading-code is copied as is
> >>
> >> IMHO, it's a good time to make style changes in the code, because
> >> current functions doesn't follow a lot of OVS coding style guidelines
> >> and we're touching them anyway.
> >>
> >
> > I agree it is a good time, but I think it should be done on a separate
> > commit,
> because if
> > something breaks (although all are style changes), it will be hard to find
> > the
> changes on git log. The entire file
> > Will be + .
>
> You already made mistakes while rebasing the copy-pasted code.
> Thorough review is required anyway. Moving of few braces will
> not change anything. 'sizeof' related changes are mostly obvious.
>
> If you wish, I could handle 'n_rxq' usage update in a separate
> patch as it's not really a style change.
>
> >
Thanks Ilya. I will send it all in v3
> >
> >>> --- /dev/null
> >>> +++ b/lib/netdev-rte-offloads.c
> >>> @@ -0,0 +1,775 @@
> >>> +/*
> >>> + * Copyright (c) 2019 Mellanox Technologies, Ltd.
> >>
> >> I'm not a layer but it seems strange to have above copyright for the
> >> file with copy-pasted code written by other people.
> >> However, I'm not sure how this should look like.
In v3 will add 2 Copyrights (original + Mellanox)
> >>
> >>> + *
> >>> + * 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://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> >> w.apache.org%2Flicenses%2FLICENSE-
> >>
> 2.0data=02%7C01%7Croniba%40mellanox.com%7C1cbceab6b5394a30
> >>
> 6a9e08d69818689d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6
> >>
> 36863627943149395sdata=GFoau2tQaeBk6KwZcleg4lZcbN%2B11WIBb
> >> Xb%2FfE50eE0%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 OF ANY KIND, either
> express
> >> or implied.
> >>> + * See the License for the specific language governing permissions and
> >>> + * limitations under the License.
> >>> + */
> >>> +#include
> >>
> >> Include of "netdev-rte-offloads.h" should be here.
> >> Blank line.
> >>
> >>> +#include
> >>
> >> Blank line.
Fixed in v3
> >>
> >>> +#include "netdev-rte-offloads.h"
> >>> +#include "netdev-provider.h"
> >>> +#include "dpif-netdev.h"
> >>
> >> Do you really need above header ?
> >>
Yes we do
> >>> +#include "cmap.h"
> >>> +#include "openvswitch/vlog.h"
> >>> +#include "openvswitch/match.h"
> >>> +#include "packets.h"
> >>> +#include "uuid.h"
> >>
> >> Above headers should go in alphabetical order.
> >>
> >> For the style guidelines please refer:
> >> Documentation/internals/contributing/coding-style.rst
> >>
> >>> +
> >>> +VLOG_DEFINE_THIS_MODULE(netdev_rte_offloads);
> >>> +
> >>> +/*
> >>> + * A mapping from ufid to dpdk rte_flow.
> >>> + */
> >>> +static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
> >>> +
> >>> +struct ufid_to_rte_flow_data {
> >>> +struct cmap_node node;
> >>> +ovs_u128 ufid;
> >>> +struct rte_flow *rte_flow;
> >>> +};
> >>> +
> >>> +
> >>> +/* Find rte_flow with @ufid */
> >>> +static struct rte_flow *
> >>> +ufid_to_rte_flow_find(const ovs_u128 *ufid) {
>
Re: [ovs-dev] [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new file
> -Original Message-
> From: Ilya Maximets
> Sent: Tuesday, February 26, 2019 4:38 PM
> To: Roni Bar Yanai ; Ophir Munk
> ; ovs-dev@openvswitch.org
> Cc: Ian Stokes ; Olga Shern ;
> Kevin Traynor ; Asaf Penso ;
> Flavio Leitner
> Subject: Re: [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new
> file
>
> On 21.02.2019 19:31, Roni Bar Yanai wrote:
> >
> >
> >> -Original Message-
> >> From: Ilya Maximets
> >> Sent: Thursday, February 21, 2019 6:20 PM
> >> To: Ophir Munk ; ovs-dev@openvswitch.org
> >> Cc: Ian Stokes ; Olga Shern
> ;
> >> Kevin Traynor ; Asaf Penso
> ;
> >> Roni Bar Yanai ; Flavio Leitner
>
> >> Subject: Re: [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new
> >> file
> >>
> >> At first, 'git am' fails to apply the patch:
> >>
> >> Applying: netdev-dpdk: Move offloading-code to a new file
> >> error: patch failed: lib/netdev-dpdk.c:4240
> >> error: lib/netdev-dpdk.c: patch does not apply
> >> .git/rebase-apply/patch:1564: new blank line at EOF.
> >> +
> >> Patch failed at 0002 netdev-dpdk: Move offloading-code to a new file
> >>
> >> I'm not sure why it complains about 'lib/netdev-dpdk.c' because the
> >> issue is the blanlk line at the end of netdev-rte-offloads.c.
> >>
> >> On 21.02.2019 15:07, Ophir Munk wrote:
> >>> From: Roni Bar Yanai
> >>>
> >>> Hardware offloading-code is moved to a new file called
> >>
> >> Just curious, why you writing "offloading-code" as a single dash
> >> separated word?
> >>
> >>> netdev-rte-offloads.c. The original offloading-code is copied as is
> >>
> >> IMHO, it's a good time to make style changes in the code, because
> >> current functions doesn't follow a lot of OVS coding style guidelines
> >> and we're touching them anyway.
> >>
> >
> > I agree it is a good time, but I think it should be done on a separate
> > commit,
> because if
> > something breaks (although all are style changes), it will be hard to find
> > the
> changes on git log. The entire file
> > Will be + .
>
> You already made mistakes while rebasing the copy-pasted code.
> Thorough review is required anyway. Moving of few braces will
> not change anything. 'sizeof' related changes are mostly obvious.
>
> If you wish, I could handle 'n_rxq' usage update in a separate
> patch as it's not really a style change.
>
Ok. Will fix all on V3.
Thanks.
> >
> >
> >>> from the netdev-dpdk.c file to the new file, where future
> >>> offloading-code should be added as well. The netdev-dpdk.c file will
> >>> remain unchanged as new offloading-code is added.
> >>>
> >>> Reviewed-by: Asaf Penso
> >>> Signed-off-by: Roni Bar Yanai
> >>> Signed-off-by: Ophir Munk
> >>> ---
> >>> lib/automake.mk | 4 +-
> >>> lib/netdev-dpdk.c | 727
> >>> +--
> >>> lib/netdev-rte-offloads.c | 775
> >> ++
> >>> lib/netdev-rte-offloads.h | 38 +++
> >>> 4 files changed, 817 insertions(+), 727 deletions(-)
> >>> create mode 100644 lib/netdev-rte-offloads.c
> >>> create mode 100644 lib/netdev-rte-offloads.h
> >>>
> >>> diff --git a/lib/automake.mk b/lib/automake.mk
> >>> index bae032b..cc5dccf 100644
> >>> --- a/lib/automake.mk
> >>> +++ b/lib/automake.mk
> >>> @@ -138,6 +138,7 @@ lib_libopenvswitch_la_SOURCES = \
> >>> lib/netdev-dpdk.h \
> >>> lib/netdev-dummy.c \
> >>> lib/netdev-provider.h \
> >>> + lib/netdev-rte-offloads.h \
> >>> lib/netdev-vport.c \
> >>> lib/netdev-vport.h \
> >>> lib/netdev-vport-private.h \
> >>> @@ -411,7 +412,8 @@ endif
> >>> if DPDK_NETDEV
> >>> lib_libopenvswitch_la_SOURCES += \
> >>> lib/dpdk.c \
> >>> - lib/netdev-dpdk.c
> >>> + lib/netdev-dpdk.c \
> >>> + lib/netdev-rte-offloads.c
> >>> else
> >>> lib_libopenvswitch_la_SOURCES += \
> >>> lib/dpdk-stub.c
> >>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >>> index 163d4ec..c4228d2 100644
> >>> --- a/lib/netdev-dpdk.c
> >>> +++ b/lib/netdev-dpdk.c
> >>> @@ -47,6 +47,7 @@
> >>> #include "dpif-netdev.h"
> >>> #include "fatal-signal.h"
> >>> #include "netdev-provider.h"
> >>> +#include "netdev-rte-offloads.h"
> >>> #include "netdev-vport.h"
> >>> #include "odp-util.h"
> >>> #include "openvswitch/dynamic-string.h"
> >>> @@ -179,17 +180,6 @@ static const struct rte_eth_conf port_conf = {
> >>> };
> >>>
> >>> /*
> >>> - * A mapping from ufid to dpdk rte_flow.
> >>> - */
> >>> -static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
> >>> -
> >>> -struct ufid_to_rte_flow_data {
> >>> -struct cmap_node node;
> >>> -ovs_u128 ufid;
> >>> -struct rte_flow *rte_flow;
> >>> -};
> >>> -
> >>> -/*
> >>> * These callbacks allow virtio-net devices to be added to vhost ports
> when
> >>> * configuration has been fully completed.
> >>> */
> >>> @@ -4240,721 +4230,6 @@ netdev_dpdk_rte_flow_create(struct
> netdev
> >> *netdev,
> >>> return flow;
> >>> }
> >>>
> >>> -/* Find rte_flow with @ufid */
> >>> -static struct rte_flow *
> >>>
Re: [ovs-dev] [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new file
On 21.02.2019 19:31, Roni Bar Yanai wrote:
>
>
>> -Original Message-
>> From: Ilya Maximets
>> Sent: Thursday, February 21, 2019 6:20 PM
>> To: Ophir Munk ; ovs-dev@openvswitch.org
>> Cc: Ian Stokes ; Olga Shern ;
>> Kevin Traynor ; Asaf Penso ;
>> Roni Bar Yanai ; Flavio Leitner
>> Subject: Re: [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new
>> file
>>
>> At first, 'git am' fails to apply the patch:
>>
>> Applying: netdev-dpdk: Move offloading-code to a new file
>> error: patch failed: lib/netdev-dpdk.c:4240
>> error: lib/netdev-dpdk.c: patch does not apply
>> .git/rebase-apply/patch:1564: new blank line at EOF.
>> +
>> Patch failed at 0002 netdev-dpdk: Move offloading-code to a new file
>>
>> I'm not sure why it complains about 'lib/netdev-dpdk.c' because the
>> issue is the blanlk line at the end of netdev-rte-offloads.c.
>>
>> On 21.02.2019 15:07, Ophir Munk wrote:
>>> From: Roni Bar Yanai
>>>
>>> Hardware offloading-code is moved to a new file called
>>
>> Just curious, why you writing "offloading-code" as a single dash
>> separated word?
>>
>>> netdev-rte-offloads.c. The original offloading-code is copied as is
>>
>> IMHO, it's a good time to make style changes in the code, because
>> current functions doesn't follow a lot of OVS coding style guidelines
>> and we're touching them anyway.
>>
>
> I agree it is a good time, but I think it should be done on a separate
> commit, because if
> something breaks (although all are style changes), it will be hard to find
> the changes on git log. The entire file
> Will be + .
You already made mistakes while rebasing the copy-pasted code.
Thorough review is required anyway. Moving of few braces will
not change anything. 'sizeof' related changes are mostly obvious.
If you wish, I could handle 'n_rxq' usage update in a separate
patch as it's not really a style change.
>
>
>>> from the netdev-dpdk.c file to the new file, where future
>>> offloading-code should be added as well. The netdev-dpdk.c file will
>>> remain unchanged as new offloading-code is added.
>>>
>>> Reviewed-by: Asaf Penso
>>> Signed-off-by: Roni Bar Yanai
>>> Signed-off-by: Ophir Munk
>>> ---
>>> lib/automake.mk | 4 +-
>>> lib/netdev-dpdk.c | 727 +--
>>> lib/netdev-rte-offloads.c | 775
>> ++
>>> lib/netdev-rte-offloads.h | 38 +++
>>> 4 files changed, 817 insertions(+), 727 deletions(-)
>>> create mode 100644 lib/netdev-rte-offloads.c
>>> create mode 100644 lib/netdev-rte-offloads.h
>>>
>>> diff --git a/lib/automake.mk b/lib/automake.mk
>>> index bae032b..cc5dccf 100644
>>> --- a/lib/automake.mk
>>> +++ b/lib/automake.mk
>>> @@ -138,6 +138,7 @@ lib_libopenvswitch_la_SOURCES = \
>>> lib/netdev-dpdk.h \
>>> lib/netdev-dummy.c \
>>> lib/netdev-provider.h \
>>> + lib/netdev-rte-offloads.h \
>>> lib/netdev-vport.c \
>>> lib/netdev-vport.h \
>>> lib/netdev-vport-private.h \
>>> @@ -411,7 +412,8 @@ endif
>>> if DPDK_NETDEV
>>> lib_libopenvswitch_la_SOURCES += \
>>> lib/dpdk.c \
>>> - lib/netdev-dpdk.c
>>> + lib/netdev-dpdk.c \
>>> + lib/netdev-rte-offloads.c
>>> else
>>> lib_libopenvswitch_la_SOURCES += \
>>> lib/dpdk-stub.c
>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>>> index 163d4ec..c4228d2 100644
>>> --- a/lib/netdev-dpdk.c
>>> +++ b/lib/netdev-dpdk.c
>>> @@ -47,6 +47,7 @@
>>> #include "dpif-netdev.h"
>>> #include "fatal-signal.h"
>>> #include "netdev-provider.h"
>>> +#include "netdev-rte-offloads.h"
>>> #include "netdev-vport.h"
>>> #include "odp-util.h"
>>> #include "openvswitch/dynamic-string.h"
>>> @@ -179,17 +180,6 @@ static const struct rte_eth_conf port_conf = {
>>> };
>>>
>>> /*
>>> - * A mapping from ufid to dpdk rte_flow.
>>> - */
>>> -static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
>>> -
>>> -struct ufid_to_rte_flow_data {
>>> -struct cmap_node node;
>>> -ovs_u128 ufid;
>>> -struct rte_flow *rte_flow;
>>> -};
>>> -
>>> -/*
>>> * These callbacks allow virtio-net devices to be added to vhost ports when
>>> * configuration has been fully completed.
>>> */
>>> @@ -4240,721 +4230,6 @@ netdev_dpdk_rte_flow_create(struct netdev
>> *netdev,
>>> return flow;
>>> }
>>>
>>> -/* Find rte_flow with @ufid */
>>> -static struct rte_flow *
>>> -ufid_to_rte_flow_find(const ovs_u128 *ufid) {
>>> -size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
>>> -struct ufid_to_rte_flow_data *data;
>>> -
>>> -CMAP_FOR_EACH_WITH_HASH (data, node, hash, _to_rte_flow) {
>>> -if (ovs_u128_equals(*ufid, data->ufid)) {
>>> -return data->rte_flow;
>>> -}
>>> -}
>>> -
>>> -return NULL;
>>> -}
>>> -
>>> -static inline void
>>> -ufid_to_rte_flow_associate(const ovs_u128 *ufid,
>>> - struct rte_flow *rte_flow) {
>>> -size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
>>> -struct ufid_to_rte_flow_data *data =
Re: [ovs-dev] [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new file
> -Original Message-
> From: Ilya Maximets
> Sent: Thursday, February 21, 2019 6:20 PM
> To: Ophir Munk ; ovs-dev@openvswitch.org
> Cc: Ian Stokes ; Olga Shern ;
> Kevin Traynor ; Asaf Penso ;
> Roni Bar Yanai ; Flavio Leitner
> Subject: Re: [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new
> file
>
> At first, 'git am' fails to apply the patch:
>
> Applying: netdev-dpdk: Move offloading-code to a new file
> error: patch failed: lib/netdev-dpdk.c:4240
> error: lib/netdev-dpdk.c: patch does not apply
> .git/rebase-apply/patch:1564: new blank line at EOF.
> +
> Patch failed at 0002 netdev-dpdk: Move offloading-code to a new file
>
> I'm not sure why it complains about 'lib/netdev-dpdk.c' because the
> issue is the blanlk line at the end of netdev-rte-offloads.c.
>
> On 21.02.2019 15:07, Ophir Munk wrote:
> > From: Roni Bar Yanai
> >
> > Hardware offloading-code is moved to a new file called
>
> Just curious, why you writing "offloading-code" as a single dash
> separated word?
>
> > netdev-rte-offloads.c. The original offloading-code is copied as is
>
> IMHO, it's a good time to make style changes in the code, because
> current functions doesn't follow a lot of OVS coding style guidelines
> and we're touching them anyway.
>
I agree it is a good time, but I think it should be done on a separate commit,
because if
something breaks (although all are style changes), it will be hard to find the
changes on git log. The entire file
Will be + .
> > from the netdev-dpdk.c file to the new file, where future
> > offloading-code should be added as well. The netdev-dpdk.c file will
> > remain unchanged as new offloading-code is added.
> >
> > Reviewed-by: Asaf Penso
> > Signed-off-by: Roni Bar Yanai
> > Signed-off-by: Ophir Munk
> > ---
> > lib/automake.mk | 4 +-
> > lib/netdev-dpdk.c | 727 +--
> > lib/netdev-rte-offloads.c | 775
> ++
> > lib/netdev-rte-offloads.h | 38 +++
> > 4 files changed, 817 insertions(+), 727 deletions(-)
> > create mode 100644 lib/netdev-rte-offloads.c
> > create mode 100644 lib/netdev-rte-offloads.h
> >
> > diff --git a/lib/automake.mk b/lib/automake.mk
> > index bae032b..cc5dccf 100644
> > --- a/lib/automake.mk
> > +++ b/lib/automake.mk
> > @@ -138,6 +138,7 @@ lib_libopenvswitch_la_SOURCES = \
> > lib/netdev-dpdk.h \
> > lib/netdev-dummy.c \
> > lib/netdev-provider.h \
> > + lib/netdev-rte-offloads.h \
> > lib/netdev-vport.c \
> > lib/netdev-vport.h \
> > lib/netdev-vport-private.h \
> > @@ -411,7 +412,8 @@ endif
> > if DPDK_NETDEV
> > lib_libopenvswitch_la_SOURCES += \
> > lib/dpdk.c \
> > - lib/netdev-dpdk.c
> > + lib/netdev-dpdk.c \
> > + lib/netdev-rte-offloads.c
> > else
> > lib_libopenvswitch_la_SOURCES += \
> > lib/dpdk-stub.c
> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> > index 163d4ec..c4228d2 100644
> > --- a/lib/netdev-dpdk.c
> > +++ b/lib/netdev-dpdk.c
> > @@ -47,6 +47,7 @@
> > #include "dpif-netdev.h"
> > #include "fatal-signal.h"
> > #include "netdev-provider.h"
> > +#include "netdev-rte-offloads.h"
> > #include "netdev-vport.h"
> > #include "odp-util.h"
> > #include "openvswitch/dynamic-string.h"
> > @@ -179,17 +180,6 @@ static const struct rte_eth_conf port_conf = {
> > };
> >
> > /*
> > - * A mapping from ufid to dpdk rte_flow.
> > - */
> > -static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
> > -
> > -struct ufid_to_rte_flow_data {
> > -struct cmap_node node;
> > -ovs_u128 ufid;
> > -struct rte_flow *rte_flow;
> > -};
> > -
> > -/*
> > * These callbacks allow virtio-net devices to be added to vhost ports when
> > * configuration has been fully completed.
> > */
> > @@ -4240,721 +4230,6 @@ netdev_dpdk_rte_flow_create(struct netdev
> *netdev,
> > return flow;
> > }
> >
> > -/* Find rte_flow with @ufid */
> > -static struct rte_flow *
> > -ufid_to_rte_flow_find(const ovs_u128 *ufid) {
> > -size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
> > -struct ufid_to_rte_flow_data *data;
> > -
> > -CMAP_FOR_EACH_WITH_HASH (data, node, hash, _to_rte_flow) {
> > -if (ovs_u128_equals(*ufid, data->ufid)) {
> > -return data->rte_flow;
> > -}
> > -}
> > -
> > -return NULL;
> > -}
> > -
> > -static inline void
> > -ufid_to_rte_flow_associate(const ovs_u128 *ufid,
> > - struct rte_flow *rte_flow) {
> > -size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
> > -struct ufid_to_rte_flow_data *data = xzalloc(sizeof(*data));
> > -
> > -/*
> > - * We should not simply overwrite an existing rte flow.
> > - * We should have deleted it first before re-adding it.
> > - * Thus, if following assert triggers, something is wrong:
> > - * the rte_flow is not destroyed.
> > - */
> > -ovs_assert(ufid_to_rte_flow_find(ufid) == NULL);
> > -
> > -data->ufid = *ufid;
Re: [ovs-dev] [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new file
At first, 'git am' fails to apply the patch:
Applying: netdev-dpdk: Move offloading-code to a new file
error: patch failed: lib/netdev-dpdk.c:4240
error: lib/netdev-dpdk.c: patch does not apply
.git/rebase-apply/patch:1564: new blank line at EOF.
+
Patch failed at 0002 netdev-dpdk: Move offloading-code to a new file
I'm not sure why it complains about 'lib/netdev-dpdk.c' because the
issue is the blanlk line at the end of netdev-rte-offloads.c.
On 21.02.2019 15:07, Ophir Munk wrote:
> From: Roni Bar Yanai
>
> Hardware offloading-code is moved to a new file called
Just curious, why you writing "offloading-code" as a single dash
separated word?
> netdev-rte-offloads.c. The original offloading-code is copied as is
IMHO, it's a good time to make style changes in the code, because
current functions doesn't follow a lot of OVS coding style guidelines
and we're touching them anyway.
> from the netdev-dpdk.c file to the new file, where future
> offloading-code should be added as well. The netdev-dpdk.c file will
> remain unchanged as new offloading-code is added.
>
> Reviewed-by: Asaf Penso
> Signed-off-by: Roni Bar Yanai
> Signed-off-by: Ophir Munk
> ---
> lib/automake.mk | 4 +-
> lib/netdev-dpdk.c | 727 +--
> lib/netdev-rte-offloads.c | 775
> ++
> lib/netdev-rte-offloads.h | 38 +++
> 4 files changed, 817 insertions(+), 727 deletions(-)
> create mode 100644 lib/netdev-rte-offloads.c
> create mode 100644 lib/netdev-rte-offloads.h
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index bae032b..cc5dccf 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -138,6 +138,7 @@ lib_libopenvswitch_la_SOURCES = \
> lib/netdev-dpdk.h \
> lib/netdev-dummy.c \
> lib/netdev-provider.h \
> + lib/netdev-rte-offloads.h \
> lib/netdev-vport.c \
> lib/netdev-vport.h \
> lib/netdev-vport-private.h \
> @@ -411,7 +412,8 @@ endif
> if DPDK_NETDEV
> lib_libopenvswitch_la_SOURCES += \
> lib/dpdk.c \
> - lib/netdev-dpdk.c
> + lib/netdev-dpdk.c \
> + lib/netdev-rte-offloads.c
> else
> lib_libopenvswitch_la_SOURCES += \
> lib/dpdk-stub.c
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 163d4ec..c4228d2 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -47,6 +47,7 @@
> #include "dpif-netdev.h"
> #include "fatal-signal.h"
> #include "netdev-provider.h"
> +#include "netdev-rte-offloads.h"
> #include "netdev-vport.h"
> #include "odp-util.h"
> #include "openvswitch/dynamic-string.h"
> @@ -179,17 +180,6 @@ static const struct rte_eth_conf port_conf = {
> };
>
> /*
> - * A mapping from ufid to dpdk rte_flow.
> - */
> -static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
> -
> -struct ufid_to_rte_flow_data {
> -struct cmap_node node;
> -ovs_u128 ufid;
> -struct rte_flow *rte_flow;
> -};
> -
> -/*
> * These callbacks allow virtio-net devices to be added to vhost ports when
> * configuration has been fully completed.
> */
> @@ -4240,721 +4230,6 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
> return flow;
> }
>
> -/* Find rte_flow with @ufid */
> -static struct rte_flow *
> -ufid_to_rte_flow_find(const ovs_u128 *ufid) {
> -size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
> -struct ufid_to_rte_flow_data *data;
> -
> -CMAP_FOR_EACH_WITH_HASH (data, node, hash, _to_rte_flow) {
> -if (ovs_u128_equals(*ufid, data->ufid)) {
> -return data->rte_flow;
> -}
> -}
> -
> -return NULL;
> -}
> -
> -static inline void
> -ufid_to_rte_flow_associate(const ovs_u128 *ufid,
> - struct rte_flow *rte_flow) {
> -size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
> -struct ufid_to_rte_flow_data *data = xzalloc(sizeof(*data));
> -
> -/*
> - * We should not simply overwrite an existing rte flow.
> - * We should have deleted it first before re-adding it.
> - * Thus, if following assert triggers, something is wrong:
> - * the rte_flow is not destroyed.
> - */
> -ovs_assert(ufid_to_rte_flow_find(ufid) == NULL);
> -
> -data->ufid = *ufid;
> -data->rte_flow = rte_flow;
> -
> -cmap_insert(_to_rte_flow,
> -CONST_CAST(struct cmap_node *, >node), hash);
> -}
> -
> -static inline void
> -ufid_to_rte_flow_disassociate(const ovs_u128 *ufid) {
> -size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
> -struct ufid_to_rte_flow_data *data;
> -
> -CMAP_FOR_EACH_WITH_HASH (data, node, hash, _to_rte_flow) {
> -if (ovs_u128_equals(*ufid, data->ufid)) {
> -cmap_remove(_to_rte_flow,
> -CONST_CAST(struct cmap_node *, >node), hash);
> -ovsrcu_postpone(free, data);
> -return;
> -}
> -}
> -
> -VLOG_WARN("ufid "UUID_FMT" is not associated with an rte flow\n",
> - UUID_ARGS((struct uuid
[ovs-dev] [PATCH v2 2/3] netdev-dpdk: Move offloading-code to a new file
From: Roni Bar Yanai
Hardware offloading-code is moved to a new file called
netdev-rte-offloads.c. The original offloading-code is copied as is
from the netdev-dpdk.c file to the new file, where future
offloading-code should be added as well. The netdev-dpdk.c file will
remain unchanged as new offloading-code is added.
Reviewed-by: Asaf Penso
Signed-off-by: Roni Bar Yanai
Signed-off-by: Ophir Munk
---
lib/automake.mk | 4 +-
lib/netdev-dpdk.c | 727 +--
lib/netdev-rte-offloads.c | 775 ++
lib/netdev-rte-offloads.h | 38 +++
4 files changed, 817 insertions(+), 727 deletions(-)
create mode 100644 lib/netdev-rte-offloads.c
create mode 100644 lib/netdev-rte-offloads.h
diff --git a/lib/automake.mk b/lib/automake.mk
index bae032b..cc5dccf 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -138,6 +138,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/netdev-dpdk.h \
lib/netdev-dummy.c \
lib/netdev-provider.h \
+ lib/netdev-rte-offloads.h \
lib/netdev-vport.c \
lib/netdev-vport.h \
lib/netdev-vport-private.h \
@@ -411,7 +412,8 @@ endif
if DPDK_NETDEV
lib_libopenvswitch_la_SOURCES += \
lib/dpdk.c \
- lib/netdev-dpdk.c
+ lib/netdev-dpdk.c \
+ lib/netdev-rte-offloads.c
else
lib_libopenvswitch_la_SOURCES += \
lib/dpdk-stub.c
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 163d4ec..c4228d2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -47,6 +47,7 @@
#include "dpif-netdev.h"
#include "fatal-signal.h"
#include "netdev-provider.h"
+#include "netdev-rte-offloads.h"
#include "netdev-vport.h"
#include "odp-util.h"
#include "openvswitch/dynamic-string.h"
@@ -179,17 +180,6 @@ static const struct rte_eth_conf port_conf = {
};
/*
- * A mapping from ufid to dpdk rte_flow.
- */
-static struct cmap ufid_to_rte_flow = CMAP_INITIALIZER;
-
-struct ufid_to_rte_flow_data {
-struct cmap_node node;
-ovs_u128 ufid;
-struct rte_flow *rte_flow;
-};
-
-/*
* These callbacks allow virtio-net devices to be added to vhost ports when
* configuration has been fully completed.
*/
@@ -4240,721 +4230,6 @@ netdev_dpdk_rte_flow_create(struct netdev *netdev,
return flow;
}
-/* Find rte_flow with @ufid */
-static struct rte_flow *
-ufid_to_rte_flow_find(const ovs_u128 *ufid) {
-size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
-struct ufid_to_rte_flow_data *data;
-
-CMAP_FOR_EACH_WITH_HASH (data, node, hash, _to_rte_flow) {
-if (ovs_u128_equals(*ufid, data->ufid)) {
-return data->rte_flow;
-}
-}
-
-return NULL;
-}
-
-static inline void
-ufid_to_rte_flow_associate(const ovs_u128 *ufid,
- struct rte_flow *rte_flow) {
-size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
-struct ufid_to_rte_flow_data *data = xzalloc(sizeof(*data));
-
-/*
- * We should not simply overwrite an existing rte flow.
- * We should have deleted it first before re-adding it.
- * Thus, if following assert triggers, something is wrong:
- * the rte_flow is not destroyed.
- */
-ovs_assert(ufid_to_rte_flow_find(ufid) == NULL);
-
-data->ufid = *ufid;
-data->rte_flow = rte_flow;
-
-cmap_insert(_to_rte_flow,
-CONST_CAST(struct cmap_node *, >node), hash);
-}
-
-static inline void
-ufid_to_rte_flow_disassociate(const ovs_u128 *ufid) {
-size_t hash = hash_bytes(ufid, sizeof(*ufid), 0);
-struct ufid_to_rte_flow_data *data;
-
-CMAP_FOR_EACH_WITH_HASH (data, node, hash, _to_rte_flow) {
-if (ovs_u128_equals(*ufid, data->ufid)) {
-cmap_remove(_to_rte_flow,
-CONST_CAST(struct cmap_node *, >node), hash);
-ovsrcu_postpone(free, data);
-return;
-}
-}
-
-VLOG_WARN("ufid "UUID_FMT" is not associated with an rte flow\n",
- UUID_ARGS((struct uuid *)ufid));
-}
-
-/*
- * To avoid individual xrealloc calls for each new element, a 'curent_max'
- * is used to keep track of current allocated number of elements. Starts
- * by 8 and doubles on each xrealloc call
- */
-struct flow_patterns {
-struct rte_flow_item *items;
-int cnt;
-int current_max;
-};
-
-struct flow_actions {
-struct rte_flow_action *actions;
-int cnt;
-int current_max;
-};
-
-static void
-dump_flow_pattern(struct rte_flow_item *item)
-{
-struct ds s;
-
-if (!VLOG_IS_DBG_ENABLED() || item->type == RTE_FLOW_ITEM_TYPE_END) {
-return;
-}
-
-ds_init();
-
-if (item->type == RTE_FLOW_ITEM_TYPE_ETH) {
-const struct rte_flow_item_eth *eth_spec = item->spec;
-const struct rte_flow_item_eth *eth_mask = item->mask;
-
-ds_put_cstr(, "rte flow eth pattern:\n");
-if (eth_spec) {
-ds_put_format(,
- " Spec: src="ETH_ADDR_FMT", dst="ETH_ADDR_FMT", "
-
