Re: [ovs-dev] [PATCH v3 1/3] dpdk: Logs to announce removal of defaults for socket-mem and limit.
On 7/16/21 3:47 PM, Stokes, Ian wrote: >> On 16/07/2021 14:19, Stokes, Ian wrote: On 15/07/2021 22:37, Rosemarie O'Riorden wrote: > Deprecate current OVS provided defaults for DPDK socket-mem and > socket-limit that are planned to be removed in OVS 2.17. At that point > DPDK defaults will be used instead. Warnings have been added to alert > users in advance. > A few very minor things in the series, otherwise they lgtm. UT passing, GHA passing. thanks. +cc for David Wilder. David, please see this patchset and as OVS will in future no longer construct the --socket-mem/socket-limit values, you might want to check that the DPDK defaults are working ok for PPC. If not, you still have time to fix them for DPDK 21.11 before OVS 2.17. >>> >>> >>> I think this patch should be applied to the master branch today in time for >> branch of the 2.16 release. Patch 2 and 3 should be left to master after 2.16 >> branches. >>> >> >> That makes sense. >> >>> @Kevin Traynor Am I ok to add you ack here? >>> >> >> Yes, thanks. >> >> Acked-by: Kevin Traynor >> > > Thanks, pushed this patch only to master with minor adjustments as flagged > below. Thanks. OVS is branched for 2.16 now, so I fixed the flagged checkpatch issue, rebased/expanded NEWS and applied the rest of the series to master. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V3 2/2] netdev-offload-dpdk: Fix vxlan vni cast-align warnings
On 7/25/21 10:54 AM, Eli Britstein wrote:
> Reported-by: Harry Van Haaren
> Fixes: 4e432d6f8128 ("netdev-offload-dpdk: Support tnl/push using vxlan encap
> attribute.")
> Fixes: e098c2f966cb ("netdev-dpdk-offload: Add vxlan pattern matching
> function.")
> Signed-off-by: Eli Britstein
> ---
> lib/netdev-offload-dpdk.c | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
Thanks! Applied to master and barnch-2.16.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V3 1/2] netdev-offload-dpdk: Fix IPv6 rewrite cast-align warning
On 7/25/21 10:54 AM, Eli Britstein wrote:
> Fixes: b6207b1d2711 ("netdev-offload-dpdk: Support offload of set IPv6
> actions.")
> Signed-off-by: Eli Britstein
> ---
> lib/netdev-offload-dpdk.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index e7913292e..4112fc3a5 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -568,8 +568,11 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
>
> ds_put_format(s, "set_ipv6_%s ", dirstr);
> if (set_ipv6) {
> +struct in6_addr addr;
> +
> ds_put_cstr(s, "ipv6_addr ");
> -ipv6_format_addr((struct in6_addr *) _ipv6->ipv6_addr, s);
> +memcpy(, set_ipv6->ipv6_addr, sizeof addr);
> +ipv6_format_addr(, s);
> ds_put_cstr(s, " ");
> }
> ds_put_cstr(s, "/ ");
>
Thanks! Applied and backported down to 2.14.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] daemon-unix: Fix leak of a fork error message.
On 7/25/21 8:06 AM, Roi Dayan wrote:
>
>
> On 2021-07-23 4:41 PM, Ilya Maximets wrote:
>> 19 bytes in 1 blocks are definitely lost in loss record 24 of 121
>> at 0x4839748: malloc (vg_replace_malloc.c:306)
>> by 0x483BD63: realloc (vg_replace_malloc.c:834)
>> by 0x521C26: xrealloc (util.c:149)
>> by 0x478F91: ds_reserve (dynamic-string.c:63)
>> by 0x47928B: ds_put_format_valist (dynamic-string.c:161)
>> by 0x47920A: ds_put_format (dynamic-string.c:142)
>> by 0x506DE5: process_status_msg (process.c:0)
>> by 0x52A6D0: fork_and_wait_for_startup (daemon-unix.c:284)
>> by 0x52A54D: daemonize_start (daemon-unix.c:453)
>> by 0x40EB3E: main (ovs-vswitchd.c:91)
>>
>> Fixes: b925336a36e6 ("daemon: restart child process if it died before
>> signaling its readiness")
>> Signed-off-by: Ilya Maximets
>> ---
>> lib/daemon-unix.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
>> index ae59ecf2c..34d45b82a 100644
>> --- a/lib/daemon-unix.c
>> +++ b/lib/daemon-unix.c
>> @@ -285,6 +285,7 @@ fork_and_wait_for_startup(int *fdp, pid_t *child_pid)
>> VLOG_ERR("fork child died before signaling startup
>> (%s)",
>> status_msg);
>> ret = -1;
>> + free(status_msg);
>> }
>> } else if (retval < 0) {
>> VLOG_FATAL("waitpid failed (%s)", ovs_strerror(errno));
>>
>
> Acked-by: Roi Dayan
Thanks! Applied and backported.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 3/3] dpif-netdev: Log flow modification in debug level
On 7/12/21 5:07 PM, Eli Britstein wrote:
> Log flow modifications to help debugging.
>
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
> lib/dpif-netdev.c | 101 +-
> 1 file changed, 55 insertions(+), 46 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 9b2b8d6d9..caed3e7f2 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2457,6 +2457,61 @@ queue_netdev_flow_put(struct dp_netdev_pmd_thread *pmd,
> struct dp_flow_offload_item *offload;
> int op;
>
> +if (OVS_UNLIKELY(!VLOG_DROP_DBG((_rl {
> +struct ds ds = DS_EMPTY_INITIALIZER;
> +struct ofpbuf key_buf, mask_buf;
> +struct odp_flow_key_parms odp_parms = {
> +.flow = >flow,
> +.mask = >wc.masks,
> +.support = dp_netdev_support,
> +};
> +
> +ofpbuf_init(_buf, 0);
> +ofpbuf_init(_buf, 0);
> +
> +odp_flow_key_from_flow(_parms, _buf);
> +odp_parms.key_buf = _buf;
> +odp_flow_key_from_mask(_parms, _buf);
> +
> +if (old_actions) {
> +ds_put_cstr(, "flow_mod: ");
> +} else {
> +ds_put_cstr(, "flow_add: ");
> +}
> +odp_format_ufid(>ufid, );
> +ds_put_cstr(, " mega_");
> +odp_format_ufid(>mega_ufid, );
> +ds_put_cstr(, " ");
> +odp_flow_format(key_buf.data, key_buf.size,
> +mask_buf.data, mask_buf.size,
> +NULL, , false);
> +if (old_actions) {
> +ds_put_cstr(, ", old_actions:");
> +format_odp_actions(, old_actions->actions, old_actions->size,
> + NULL);
> +}
> +ds_put_cstr(, ", actions:");
> +format_odp_actions(, actions, actions_len, NULL);
> +
> +VLOG_DBG("%s", ds_cstr());
> +
> +ofpbuf_uninit(_buf);
> +ofpbuf_uninit(_buf);
> +
> +/* Add a printout of the actual match installed. */
> +struct match m;
> +ds_clear();
> +ds_put_cstr(, "flow match: ");
> +miniflow_expand(>cr.flow.mf, );
> +miniflow_expand(>cr.mask->mf, );
> +memset(_md, 0, sizeof m.tun_md);
> +match_format(, NULL, , OFP_DEFAULT_PRIORITY);
> +
> +VLOG_DBG("%s", ds_cstr());
> +
> +ds_destroy();
> +}
> +
It make sense to print. However, I don't think that this generic
log belongs to queue_netdev_flow_put() function. It shouldn't be
there. Suggesting to create a separate function for logging and
call it in two places after queue_netdev_flow_put().
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 2/3] dpif-netdev: Fix offloads of modified flows
On 7/12/21 5:07 PM, Eli Britstein wrote:
> Association of a mark to a flow is done as part of its offload handling,
> in the offloading thread. However, the PMD thread specifies whether an
> offload request is an "add" or "modify" by the association of a mark to
> the flow.
> This is exposed to a race condition. A flow might be created with
> actions that cannot be fully offloaded, for example flooding (before MAC
> learning), and later modified to have actions that can be fully
> offloaded. If the two requests are queued before the offload thread
> handling, they are both marked as "add". When the offload thread handles
> them, the first request is partially offloaded, and the second one is
> ignored as the flow is already considered as offloaded.
>
> Fix it by specifying add/modify of an offload request by the actual flow
> state change, without relying on the mark.
>
> Fixes: 3c7330ebf036 ("netdev-offload-dpdk: Support offload of output action.")
> Signed-off-by: Eli Britstein
> Reviewed-by: Gaetan Rivet
> ---
> lib/dpif-netdev.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 21b0e025d..9b2b8d6d9 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2451,7 +2451,8 @@ static void
> queue_netdev_flow_put(struct dp_netdev_pmd_thread *pmd,
>struct dp_netdev_flow *flow, struct match *match,
>const struct nlattr *actions, size_t actions_len,
> - odp_port_t orig_in_port)
> + odp_port_t orig_in_port,
> + const struct dp_netdev_actions *old_actions)
> {
> struct dp_flow_offload_item *offload;
> int op;
> @@ -2467,11 +2468,9 @@ queue_netdev_flow_put(struct dp_netdev_pmd_thread *pmd,
> ovsthread_once_done(_thread_once);
> }
>
> -if (flow->mark != INVALID_FLOW_MARK) {
> -op = DP_NETDEV_FLOW_OFFLOAD_OP_MOD;
> -} else {
> -op = DP_NETDEV_FLOW_OFFLOAD_OP_ADD;
> -}
> +op = old_actions
> +? DP_NETDEV_FLOW_OFFLOAD_OP_MOD
> +: DP_NETDEV_FLOW_OFFLOAD_OP_ADD;
The change looks good to me in general, but I think it's better to
just directly pass DP_NETDEV_FLOW_OFFLOAD_OP_MOD/ADD to the function
instead of passing 'old_actions' pointer that is not really used here.
I see that you will use it in the next patch of the set. I'll reply
to patch #3.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/3] dpif-netdev: Do not flush PMD offloads on reload
On 7/25/21 8:40 AM, Eli Britstein wrote:
>
> On 7/23/2021 9:00 PM, Ilya Maximets wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 7/12/21 5:07 PM, Eli Britstein wrote:
>>> Before flushing offloads of a removed port was supported by [1], it was
>>> necessary to flush the 'marks'. In doing so, all offloads of the PMD are
>>> removed, include the ones that are not related to the removed port and
>>> that are not modified following this removal. As a result such flows are
>>> evicted from being offloaded, and won't resume offloading.
>>>
>>> As PMD offload flush is not necessary, avoid it.
>>>
>>> [1] 62d1c28e9ce0 ("dpif-netdev: Flush offload rules upon port deletion.")
>>>
>>> Signed-off-by: Eli Britstein
>>> Reviewed-by: Gaetan Rivet
>>> ---
>> Is my understanding here correct:
>> On a port deletion netdev_fow_flush() will remove flows from HW and
>> offloading layer. Later, ofproto will request to remove flows from
>> the datapath. flow marks will be freed, but actual netdev_flow_del()
>> will fail, because netdev-offload already removed these flows. But
>> we do not really care about this failure. Right?
>
> That is correct.
>
> It was also the same before above [1], but then in the race condition, the
> offload memory was leaked, and the offloads themselves were either not
> destroyed (e.g. leaked) or removed by the PMD, which was PMD dependent.
>
> [1] resolved the leaking, but didn't change the mentioned failure behavior.
>
> This commit doesn't prevents this failure either, but removes the code that
> destroys offloads of flows that should not be destroyed.
Make sense. Thanks!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] ofproto-dpif-xlate: fix zone set from non-frozen-metadata fields
Re-adding the list and people from CC.
On 6/27/21 8:16 AM, 贺鹏 wrote:
> Hi, Ilya,
>
> sorry for replying so late, being really busy this month.
Busy time for everyone.
BTW, I'm taking some time off next week. Maybe someone else will be able
to take a look at the changes while I'm not here. My replies inline.
> I've read through your comments. I personally prefer to keep the
> number of megaflows as few as possible, as megaflow is becoming
> expensive as the size of flow struct is large.
> below are my experiments.
>
> Ilya Maximets 于2021年5月22日周六 上午3:50写道：
>>
>> On 5/21/21 5:00 AM, 贺鹏 wrote:
>>> Hi, Ilya
>>>
>>>
>>>
>>> Ilya Maximets 于2021年5月19日周三 下午8:50写道：
On 2/27/21 10:34 AM, Peng He wrote:
> CT zone could be set from a field that is not included in frozen
> metadata. Consider the example rules which are typically seen in
> OpenStack security group rules:
>
> priority=100,in_port=1,tcp,ct_state=-trk,action=ct(zone=5,table=0)
> priority=100,in_port=1,tcp,ct_state=+trk,action=ct(commit,zone=NXM_NX_CT_ZONE[]),2
>
> The zone is set from the first rule's ct action. These two rules will
> generate two megaflows: the first one uses zone=5 to query the CT module,
> the second one sets the zone-id from the first megaflow and commit to CT.
>
> The current implementation will generate a megaflow that does not use
> ct_zone=5 as a match, but directly commit into the ct using zone=5, as
> zone is
> set by an Imm not a field.
>
> Consider a situation that one changes the zone id (for example to 15)
> in the first rule, however, still keep the second rule unchanged. During
> this change, there is traffic hitting the two generated megaflows, the
> revaldiator would revalidate all megaflows, however, the revalidator will
> not change the second megaflow, because zone=5 is recorded in the
> megaflow, so the xlate will still translate the commit action into zone=5,
> and the new traffic will still commit to CT as zone=5, not zone=15,
> resulting in taffic drops and other issues.
>
> Just like OVS set-field convention, if a field X is set by Y
> (Y is a variable not an Imm), we should also mask Y as a match
> in the generated megaflow. An exception is that if the zone-id is
> set by the field that is included in the frozen state (i.e. regs) and this
> upcall is a resume of a thawed xlate, the un-wildcarding can be skipped,
> as the recirc_id is a hash of the values in these fields, and it will
> change
> following the changes of these fields. When the recirc_id changes,
> all megaflows with the old recirc id will be invalid later.
>
> Fixes: 07659514c3 ("Add support for connection tracking.")
> Reported-by: Sai Su
> Signed-off-by: Peng He
> ---
> include/openvswitch/meta-flow.h | 1 +
> lib/meta-flow.c | 13 ++
> ofproto/ofproto-dpif-xlate.c| 12 +
> tests/system-traffic.at | 45 +
> 4 files changed, 71 insertions(+)
>
> diff --git a/include/openvswitch/meta-flow.h
> b/include/openvswitch/meta-flow.h
> index 95e52e358..045dce8f5 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -2305,6 +2305,7 @@ void mf_set_flow_value_masked(const struct mf_field
> *,
>const union mf_value *mask,
>struct flow *);
> bool mf_is_tun_metadata(const struct mf_field *);
> +bool mf_is_frozen_metadata(const struct mf_field *);
> bool mf_is_pipeline_field(const struct mf_field *);
> bool mf_is_set(const struct mf_field *, const struct flow *);
> void mf_mask_field(const struct mf_field *, struct flow_wildcards *);
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index c808d205d..e03cd8d0c 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -1788,6 +1788,19 @@ mf_is_tun_metadata(const struct mf_field *mf)
> mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS;
> }
>
> +bool
> +mf_is_frozen_metadata(const struct mf_field *mf)
> +{
> +if (mf->id >= MFF_TUN_ID && mf->id <= MFF_IN_PORT_OXM) {
> +return true;
> +}
> +
> +if (mf->id >= MFF_REG0 && mf->id < MFF_ETH_SRC) {
> +return true;
> +}
> +return false;
> +}
> +
> bool
> mf_is_pipeline_field(const struct mf_field *mf)
> {
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7108c8a30..14d00db1e 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -6195,6 +6195,18 @@ compose_conntrack_action(struct xlate_ctx *ctx,
> struct ofpact_conntrack *ofc,
>
> if (ofc->zone_src.field) {
> zone =
[ovs-dev] [PATCH V3 2/2] netdev-offload-dpdk: Fix vxlan vni cast-align warnings
Reported-by: Harry Van Haaren
Fixes: 4e432d6f8128 ("netdev-offload-dpdk: Support tnl/push using vxlan encap
attribute.")
Fixes: e098c2f966cb ("netdev-dpdk-offload: Add vxlan pattern matching
function.")
Signed-off-by: Eli Britstein
---
lib/netdev-offload-dpdk.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 4112fc3a5..f6706ee0c 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -375,15 +375,19 @@ dump_flow_pattern(struct ds *s,
} else if (item->type == RTE_FLOW_ITEM_TYPE_VXLAN) {
const struct rte_flow_item_vxlan *vxlan_spec = item->spec;
const struct rte_flow_item_vxlan *vxlan_mask = item->mask;
+ovs_be32 spec_vni, mask_vni;
ds_put_cstr(s, "vxlan ");
if (vxlan_spec) {
if (!vxlan_mask) {
vxlan_mask = _flow_item_vxlan_mask;
}
+spec_vni = get_unaligned_be32(ALIGNED_CAST(ovs_be32 *,
+ vxlan_spec->vni));
+mask_vni = get_unaligned_be32(ALIGNED_CAST(ovs_be32 *,
+ vxlan_mask->vni));
DUMP_PATTERN_ITEM(vxlan_mask->vni, "vni", "%"PRIu32,
- ntohl(*(ovs_be32 *) vxlan_spec->vni) >> 8,
- ntohl(*(ovs_be32 *) vxlan_mask->vni) >> 8);
+ ntohl(spec_vni) >> 8, ntohl(mask_vni) >> 8);
}
ds_put_cstr(s, "/ ");
} else {
@@ -417,8 +421,11 @@ dump_vxlan_encap(struct ds *s, const struct rte_flow_item
*items)
ds_put_format(s, "set vxlan ip-version %s ",
ipv4 ? "ipv4" : ipv6 ? "ipv6" : "ERR");
if (vxlan) {
-ds_put_format(s, "vni %"PRIu32" ",
- ntohl(*(ovs_be32 *) vxlan->vni) >> 8);
+ovs_be32 vni;
+
+vni = get_unaligned_be32(ALIGNED_CAST(ovs_be32 *,
+ vxlan->vni));
+ds_put_format(s, "vni %"PRIu32" ", ntohl(vni) >> 8);
}
if (udp) {
ds_put_format(s, "udp-src %"PRIu16" udp-dst %"PRIu16" ",
@@ -1002,9 +1009,9 @@ parse_vxlan_match(struct flow_patterns *patterns,
vx_spec = xzalloc(sizeof *vx_spec);
vx_mask = xzalloc(sizeof *vx_mask);
-put_unaligned_be32((ovs_be32 *) vx_spec->vni,
+put_unaligned_be32(ALIGNED_CAST(ovs_be32 *, vx_spec->vni),
htonl(ntohll(match->flow.tunnel.tun_id) << 8));
-put_unaligned_be32((ovs_be32 *) vx_mask->vni,
+put_unaligned_be32(ALIGNED_CAST(ovs_be32 *, vx_mask->vni),
htonl(ntohll(match->wc.masks.tunnel.tun_id) << 8));
consumed_masks->tunnel.tun_id = 0;
--
2.25.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH V3 1/2] netdev-offload-dpdk: Fix IPv6 rewrite cast-align warning
Fixes: b6207b1d2711 ("netdev-offload-dpdk: Support offload of set IPv6
actions.")
Signed-off-by: Eli Britstein
---
lib/netdev-offload-dpdk.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index e7913292e..4112fc3a5 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -568,8 +568,11 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
ds_put_format(s, "set_ipv6_%s ", dirstr);
if (set_ipv6) {
+struct in6_addr addr;
+
ds_put_cstr(s, "ipv6_addr ");
-ipv6_format_addr((struct in6_addr *) _ipv6->ipv6_addr, s);
+memcpy(, set_ipv6->ipv6_addr, sizeof addr);
+ipv6_format_addr(, s);
ds_put_cstr(s, " ");
}
ds_put_cstr(s, "/ ");
--
2.25.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V2 1/3] dpif-netdev: Do not flush PMD offloads on reload
On 7/23/2021 9:00 PM, Ilya Maximets wrote:
External email: Use caution opening links or attachments
On 7/12/21 5:07 PM, Eli Britstein wrote:
Before flushing offloads of a removed port was supported by [1], it was
necessary to flush the 'marks'. In doing so, all offloads of the PMD are
removed, include the ones that are not related to the removed port and
that are not modified following this removal. As a result such flows are
evicted from being offloaded, and won't resume offloading.
As PMD offload flush is not necessary, avoid it.
[1] 62d1c28e9ce0 ("dpif-netdev: Flush offload rules upon port deletion.")
Signed-off-by: Eli Britstein
Reviewed-by: Gaetan Rivet
---
Is my understanding here correct:
On a port deletion netdev_fow_flush() will remove flows from HW and
offloading layer. Later, ofproto will request to remove flows from
the datapath. flow marks will be freed, but actual netdev_flow_del()
will fail, because netdev-offload already removed these flows. But
we do not really care about this failure. Right?
That is correct.
It was also the same before above [1], but then in the race condition,
the offload memory was leaked, and the offloads themselves were either
not destroyed (e.g. leaked) or removed by the PMD, which was PMD dependent.
[1] resolved the leaking, but didn't change the mentioned failure behavior.
This commit doesn't prevents this failure either, but removes the code
that destroys offloads of flows that should not be destroyed.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-linux.c : nl_sock_listen_all_nsid triggers NULL deref.
On 2021-05-19 6:57 PM, lin huang wrote:
netdev-linux.c : nl_sock_listen_all_nsid triggers NULL deref.
Signed-off-by: miter
---
lib/netdev-linux.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 15b25084b..0994044ec 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -634,7 +634,9 @@ netdev_linux_notify_sock(void)
}
}
}
-nl_sock_listen_all_nsid(sock, true);
+if (sock) {
+nl_sock_listen_all_nsid(sock, true);
+ }
ovsthread_once_done();
}
--
2.31.1
___
dev mailing list
d...@openvswitch.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=04%7C01%7Croid%40nvidia.com%7C215ca7c887024f7daef208d91adecd05%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637570366499393944%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=Iz3GaO7wuuEtcEmKdN5tQHdF48pyhARZoJvw1HGQb9E%3Dreserved=0
Hi,
Can you fix the checkpatch warning?
Thanks,
Roi
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] daemon-unix: Fix leak of a fork error message.
On 2021-07-23 4:41 PM, Ilya Maximets wrote:
19 bytes in 1 blocks are definitely lost in loss record 24 of 121
at 0x4839748: malloc (vg_replace_malloc.c:306)
by 0x483BD63: realloc (vg_replace_malloc.c:834)
by 0x521C26: xrealloc (util.c:149)
by 0x478F91: ds_reserve (dynamic-string.c:63)
by 0x47928B: ds_put_format_valist (dynamic-string.c:161)
by 0x47920A: ds_put_format (dynamic-string.c:142)
by 0x506DE5: process_status_msg (process.c:0)
by 0x52A6D0: fork_and_wait_for_startup (daemon-unix.c:284)
by 0x52A54D: daemonize_start (daemon-unix.c:453)
by 0x40EB3E: main (ovs-vswitchd.c:91)
Fixes: b925336a36e6 ("daemon: restart child process if it died before signaling its
readiness")
Signed-off-by: Ilya Maximets
---
lib/daemon-unix.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
index ae59ecf2c..34d45b82a 100644
--- a/lib/daemon-unix.c
+++ b/lib/daemon-unix.c
@@ -285,6 +285,7 @@ fork_and_wait_for_startup(int *fdp, pid_t *child_pid)
VLOG_ERR("fork child died before signaling startup (%s)",
status_msg);
ret = -1;
+free(status_msg);
}
} else if (retval < 0) {
VLOG_FATAL("waitpid failed (%s)", ovs_strerror(errno));
Acked-by: Roi Dayan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
