<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/openvswitch.git/tests/ofproto-dpif.at, branch master</title>
<subtitle>github.com: openvswitch/ovs.git
</subtitle>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/'/>
<entry>
<title>ovs-dpctl: Add new command dpctl/ct-[sg]et-sweep-interval.</title>
<updated>2023-04-06T20:59:25+00:00</updated>
<author>
<name>Paolo Valerio</name>
<email>pvalerio@redhat.com</email>
</author>
<published>2023-04-06T10:10:22+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=9fa612959cfb37115aac2678f10f1538b755c797'/>
<id>9fa612959cfb37115aac2678f10f1538b755c797</id>
<content type='text'>
Since 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists
with rculists.") the sweep interval changed as well as the constraints
related to the sweeper.
Being able to change the default reschedule time may be convenient in
some conditions, like debugging.
This patch introduces new commands allowing to get and set the sweep
interval in ms.

Signed-off-by: Paolo Valerio &lt;pvalerio@redhat.com&gt;
Reviewed-by: Simon Horman &lt;simon.horman@corigine.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Since 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists
with rculists.") the sweep interval changed as well as the constraints
related to the sweeper.
Being able to change the default reschedule time may be convenient in
some conditions, like debugging.
This patch introduces new commands allowing to get and set the sweep
interval in ms.

Signed-off-by: Paolo Valerio &lt;pvalerio@redhat.com&gt;
Reviewed-by: Simon Horman &lt;simon.horman@corigine.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ofproto-dpif-xlate: Always mask ip proto field.</title>
<updated>2023-04-06T11:17:15+00:00</updated>
<author>
<name>Aaron Conole</name>
<email>aconole@redhat.com</email>
</author>
<published>2023-03-31T21:17:27+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=9d840923d32124fe427de76e8234c49d64e4bb77'/>
<id>9d840923d32124fe427de76e8234c49d64e4bb77</id>
<content type='text'>
The ofproto layer currently treats nw_proto field as overloaded to mean
both that a proper nw layer exists, as well as the value contained in
the header for the nw proto.  However, this is incorrect behavior as
relevant standards permit that any value, including '0' should be treated
as a valid value.

Because of this overload, when the ofproto layer builds action list for
a packet with nw_proto of 0, it won't build the complete action list that
we expect to be built for the packet.  That will cause a bad behavior
where all packets passing the datapath will fall into an incomplete
action set.

The fix here is to unwildcard nw_proto, allowing us to preserve setting
actions for protocols which we know have support for the actions we
program.  This means that a traffic which contains nw_proto == 0 cannot
cause connectivity breakage with other traffic on the link.

Reported-by: David Marchand &lt;dmarchand@redhat.com&gt;
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2134873
Acked-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
Signed-off-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The ofproto layer currently treats nw_proto field as overloaded to mean
both that a proper nw layer exists, as well as the value contained in
the header for the nw proto.  However, this is incorrect behavior as
relevant standards permit that any value, including '0' should be treated
as a valid value.

Because of this overload, when the ofproto layer builds action list for
a packet with nw_proto of 0, it won't build the complete action list that
we expect to be built for the packet.  That will cause a bad behavior
where all packets passing the datapath will fall into an incomplete
action set.

The fix here is to unwildcard nw_proto, allowing us to preserve setting
actions for protocols which we know have support for the actions we
program.  This means that a traffic which contains nw_proto == 0 cannot
cause connectivity breakage with other traffic on the link.

Reported-by: David Marchand &lt;dmarchand@redhat.com&gt;
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2134873
Acked-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
Signed-off-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>odp-util: Make odp_flow_key_from_flow__ nlattr order the same as the kernel.</title>
<updated>2023-02-08T23:32:55+00:00</updated>
<author>
<name>Eelco Chaudron</name>
<email>echaudro@redhat.com</email>
</author>
<published>2023-02-07T14:06:07+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=d57299fc6cb3fc802faaa56e41514f28b87477de'/>
<id>d57299fc6cb3fc802faaa56e41514f28b87477de</id>
<content type='text'>
Make the order of the Netlink attributes for odp_flow_key_from_flow__()
the same as the kernel will return them.

This will make sure the attributes displayed in the dpctl/dump-flows
output appear in the same order for all datapath.

Signed-off-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Acked-by: Roi Dayan &lt;roid@nvidia.com&gt;
Reviewed-by: Simon Horman &lt;simon.horman@corigine.com&gt;
Tested-by: Simon Horman &lt;simon.horman@corigine.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Make the order of the Netlink attributes for odp_flow_key_from_flow__()
the same as the kernel will return them.

This will make sure the attributes displayed in the dpctl/dump-flows
output appear in the same order for all datapath.

Signed-off-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Acked-by: Roi Dayan &lt;roid@nvidia.com&gt;
Reviewed-by: Simon Horman &lt;simon.horman@corigine.com&gt;
Tested-by: Simon Horman &lt;simon.horman@corigine.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>lldp: Fix bugs when parsing malformed AutoAttach.</title>
<updated>2022-12-20T16:26:25+00:00</updated>
<author>
<name>Qian Chen</name>
<email>cq674350529@163.com</email>
</author>
<published>2022-12-20T14:36:08+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=7490f281f09a8455c48e19b0cf1b99ab758ee4f4'/>
<id>7490f281f09a8455c48e19b0cf1b99ab758ee4f4</id>
<content type='text'>
The OVS LLDP implementation includes support for AutoAttach standard, which
the 'upstream' lldpd project does not include.  As part of adding this
support, the message parsing for these TLVs did not include proper length
checks for the LLDP_TLV_AA_ELEMENT_SUBTYPE and the
LLDP_TLV_AA_ISID_VLAN_ASGNS_SUBTYPE elements.  The result is that a message
without a proper boundary will cause an overread of memory, and lead to
undefined results, including crashes or other unidentified behavior.

The fix is to introduce proper bounds checking for these elements.  Introduce
a unit test to ensure that we have some proper rejection in this code
base in the future.

Fixes: be53a5c447c3 ("auto-attach: Initial support for Auto-Attach standard")
Signed-off-by: Qian Chen &lt;cq674350529@163.com&gt;
Co-authored-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The OVS LLDP implementation includes support for AutoAttach standard, which
the 'upstream' lldpd project does not include.  As part of adding this
support, the message parsing for these TLVs did not include proper length
checks for the LLDP_TLV_AA_ELEMENT_SUBTYPE and the
LLDP_TLV_AA_ISID_VLAN_ASGNS_SUBTYPE elements.  The result is that a message
without a proper boundary will cause an overread of memory, and lead to
undefined results, including crashes or other unidentified behavior.

The fix is to introduce proper bounds checking for these elements.  Introduce
a unit test to ensure that we have some proper rejection in this code
base in the future.

Fixes: be53a5c447c3 ("auto-attach: Initial support for Auto-Attach standard")
Signed-off-by: Qian Chen &lt;cq674350529@163.com&gt;
Co-authored-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>mac-learning: Fix learned fdb entries not age out issue.</title>
<updated>2022-11-02T18:36:38+00:00</updated>
<author>
<name>Lin Huang</name>
<email>linhuang@ruijie.com.cn</email>
</author>
<published>2022-10-23T04:58:55+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=f1eb850aea833c5fd0cc106020184b0db63d7a30'/>
<id>f1eb850aea833c5fd0cc106020184b0db63d7a30</id>
<content type='text'>
After user add a static fdb entry, the get_lru() function will always
return the static fdb entry.  That's normal fdb entries will not age
out through mac_learning_run().

Fix the issue by modify the get_lru() function to check the
entry-&gt;expires field and not return the entry which entry-&gt;expires is
MAC_ENTRY_AGE_STATIC_ENTRY.

Adding a unit test for this.

Fixes: ccc24fc88d59 ("ofproto-dpif: APIs and CLI option to add/delete static fdb entry.")
Acked-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Tested-by: Zhang Yuhuang &lt;zhangyuhuang@ruijie.com.cn&gt;
Signed-off-by: Lin Huang &lt;linhuang@ruijie.com.cn&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
After user add a static fdb entry, the get_lru() function will always
return the static fdb entry.  That's normal fdb entries will not age
out through mac_learning_run().

Fix the issue by modify the get_lru() function to check the
entry-&gt;expires field and not return the entry which entry-&gt;expires is
MAC_ENTRY_AGE_STATIC_ENTRY.

Adding a unit test for this.

Fixes: ccc24fc88d59 ("ofproto-dpif: APIs and CLI option to add/delete static fdb entry.")
Acked-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Tested-by: Zhang Yuhuang &lt;zhangyuhuang@ruijie.com.cn&gt;
Signed-off-by: Lin Huang &lt;linhuang@ruijie.com.cn&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>bond: Avoid deadlock while updating post recirculation rules.</title>
<updated>2022-09-16T11:58:42+00:00</updated>
<author>
<name>Ilya Maximets</name>
<email>i.maximets@ovn.org</email>
</author>
<published>2022-09-13T19:08:52+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=586adfd047cbbbf5fae329180adbce1f4f4eb1db'/>
<id>586adfd047cbbbf5fae329180adbce1f4f4eb1db</id>
<content type='text'>
If the PACKET_OUT from controller ends up with sending packet to
a bond interface, the main thread will take locks in the following
order:
  handle_openflow
  --&gt; take ofproto_mutex
  handle_packet_out
  packet_xlate
  output_normal
  bond_update_post_recirc_rules
  --&gt; take rwlock in bond.c

If at the same time revalidator thread is processing other packet
with the output to the same bond:
  xlate_actions
  output_normal
  bond_update_post_recirc_rules
  --&gt; take rwlock in bond.c
  update_recirc_rules
  ofproto_dpif_add_internal_flow
  ofproto_flow_mod
  --&gt; take ofproto_mutex

So, it is possible for these 2 threads to lock each other by
taking one lock and waiting for another thread to release the
second lock.

It is also possible for the main thread to lock itself up by trying
to acquire ofproto_mutex for the second time, if it will actually
proceed with update_recirc_rules() after taking the bond rwlock.

The problem appears to be that bond_update_post_recirc_rules()
is called during the flow translation even if side effects are
prohibited, which is the case for openflow PACKET_OUT handling.

Skipping actual flow updates during the flow translation if
side effects are disabled to avoid the deadlock.

Since flows are not installed now when actions translated for
very first packet, installing initial flows in bond_reconfigure().
This will cover the case of allocating a new recirc_id.

Also checking if we need to update flows in bond_run() to cover
link state changes.

Regression test is added to catch the double lock case.

Reported-at: https://github.com/openvswitch/ovs-issues/issues/259
Reported-by: Daniel Ding &lt;zhihui.ding@easystack.cn&gt;
Fixes: adcf00ba35a0 ("ofproto/bond: Implement bond megaflow using recirculation")
Acked-by: Mike Pattrick &lt;mkp@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
If the PACKET_OUT from controller ends up with sending packet to
a bond interface, the main thread will take locks in the following
order:
  handle_openflow
  --&gt; take ofproto_mutex
  handle_packet_out
  packet_xlate
  output_normal
  bond_update_post_recirc_rules
  --&gt; take rwlock in bond.c

If at the same time revalidator thread is processing other packet
with the output to the same bond:
  xlate_actions
  output_normal
  bond_update_post_recirc_rules
  --&gt; take rwlock in bond.c
  update_recirc_rules
  ofproto_dpif_add_internal_flow
  ofproto_flow_mod
  --&gt; take ofproto_mutex

So, it is possible for these 2 threads to lock each other by
taking one lock and waiting for another thread to release the
second lock.

It is also possible for the main thread to lock itself up by trying
to acquire ofproto_mutex for the second time, if it will actually
proceed with update_recirc_rules() after taking the bond rwlock.

The problem appears to be that bond_update_post_recirc_rules()
is called during the flow translation even if side effects are
prohibited, which is the case for openflow PACKET_OUT handling.

Skipping actual flow updates during the flow translation if
side effects are disabled to avoid the deadlock.

Since flows are not installed now when actions translated for
very first packet, installing initial flows in bond_reconfigure().
This will cover the case of allocating a new recirc_id.

Also checking if we need to update flows in bond_run() to cover
link state changes.

Regression test is added to catch the double lock case.

Reported-at: https://github.com/openvswitch/ovs-issues/issues/259
Reported-by: Daniel Ding &lt;zhihui.ding@easystack.cn&gt;
Fixes: adcf00ba35a0 ("ofproto/bond: Implement bond megaflow using recirculation")
Acked-by: Mike Pattrick &lt;mkp@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ofproto-dpif-trace: add --name option for ofproto/trace.</title>
<updated>2022-09-15T12:39:28+00:00</updated>
<author>
<name>Nobuhiro MIKI</name>
<email>nmiki@yahoo-corp.jp</email>
</author>
<published>2022-08-26T07:48:53+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=a93d0b74dd0a4519e04441144756ffb056b4e8c5'/>
<id>a93d0b74dd0a4519e04441144756ffb056b4e8c5</id>
<content type='text'>
Most of commands in ovs-ofctl and ovs-appctl can display port names
instead of port numbers by using --names option. This change adds
similar functionality to ofproto/trace.

For backward compatibility, the default behavior is the same as
before.

Acked-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Nobuhiro MIKI &lt;nmiki@yahoo-corp.jp&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Most of commands in ovs-ofctl and ovs-appctl can display port names
instead of port numbers by using --names option. This change adds
similar functionality to ofproto/trace.

For backward compatibility, the default behavior is the same as
before.

Acked-by: Aaron Conole &lt;aconole@redhat.com&gt;
Signed-off-by: Nobuhiro MIKI &lt;nmiki@yahoo-corp.jp&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>tests: Fix tests with GNU grep 3.8.</title>
<updated>2022-09-14T11:24:27+00:00</updated>
<author>
<name>Andreas Stieger</name>
<email>Andreas.Stieger@gmx.de</email>
</author>
<published>2022-09-12T19:38:46+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=75c355adc057be47323ae56b5bfb3ff6b799d95f'/>
<id>75c355adc057be47323ae56b5bfb3ff6b799d95f</id>
<content type='text'>
GNU grep 3.8 started to emit warnings when invoking egrep/fgrep. In some
cases this breaks tests that check stderr. Replace the commands with
their grep -E and grep -F counterparts throughout.

Reported-at: https://bugzilla.opensuse.org/show_bug.cgi?id=1203239
Submitted-at: https://github.com/openvswitch/ovs/pull/395
Signed-off-by: Andreas Stieger &lt;Andreas.Stieger@gmx.de&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
GNU grep 3.8 started to emit warnings when invoking egrep/fgrep. In some
cases this breaks tests that check stderr. Replace the commands with
their grep -E and grep -F counterparts throughout.

Reported-at: https://bugzilla.opensuse.org/show_bug.cgi?id=1203239
Submitted-at: https://github.com/openvswitch/ovs/pull/395
Signed-off-by: Andreas Stieger &lt;Andreas.Stieger@gmx.de&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ofproto-dpif-xlate: Fix error messages for nonexistent ports/recirc_ids.</title>
<updated>2022-09-09T17:03:09+00:00</updated>
<author>
<name>Ilya Maximets</name>
<email>i.maximets@ovn.org</email>
</author>
<published>2022-08-31T10:25:51+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=49efc63ad8c9ef14248c82dceecf39377619c515'/>
<id>49efc63ad8c9ef14248c82dceecf39377619c515</id>
<content type='text'>
If tnl_port_should_receive() is false, we're looking for a normal
port, not a tunnel one.  And it's better to print recirculation IDs
in hex since they are typically printed this way in flow dumps.

Fixes: d40533fc820c ("odp-util: Improve log messages and error reporting for Netlink parsing.")
Acked-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
If tnl_port_should_receive() is false, we're looking for a normal
port, not a tunnel one.  And it's better to print recirculation IDs
in hex since they are typically printed this way in flow dumps.

Fixes: d40533fc820c ("odp-util: Improve log messages and error reporting for Netlink parsing.")
Acked-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>ofproto-dpif-xlate: Optimize datapath action set by removing last clone action.</title>
<updated>2022-09-09T14:54:58+00:00</updated>
<author>
<name>Eelco Chaudron</name>
<email>echaudro@redhat.com</email>
</author>
<published>2022-07-26T09:40:27+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openvswitch.git/commit/?id=4f5decf4ab3fd23a68116a7cb2052d209f08bf56'/>
<id>4f5decf4ab3fd23a68116a7cb2052d209f08bf56</id>
<content type='text'>
When OFPROTO non-reversible actions are translated to data plane
actions, the only thing looked at is if there are more actions
pending. If this is the case, the action is encapsulated in a
clone().

This could lead to unnecessary clones if no meaningful data
plane actions are added. For example, the register pop in the
included test case.

The best solution would probably be to build the full action
path and determine if the clone is needed. However, this would
be a huge change in the existing design, so for now, we just try
to optimize the generated datapath flow. We can revisit this
later, as some of the pending CT issues might need this rework.

Fixes: feee58b9587f ("ofproto-dpif-xlate: Keep track of the last action")
Fixes: dadd8357f224 ("ofproto-dpif: Fix issue with non-reversible actions on a patch ports.")
Acked-by: Ales Musil &lt;amusil@redhat.com&gt;
Signed-off-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When OFPROTO non-reversible actions are translated to data plane
actions, the only thing looked at is if there are more actions
pending. If this is the case, the action is encapsulated in a
clone().

This could lead to unnecessary clones if no meaningful data
plane actions are added. For example, the register pop in the
included test case.

The best solution would probably be to build the full action
path and determine if the clone is needed. However, this would
be a huge change in the existing design, so for now, we just try
to optimize the generated datapath flow. We can revisit this
later, as some of the pending CT issues might need this rework.

Fixes: feee58b9587f ("ofproto-dpif-xlate: Keep track of the last action")
Fixes: dadd8357f224 ("ofproto-dpif: Fix issue with non-reversible actions on a patch ports.")
Acked-by: Ales Musil &lt;amusil@redhat.com&gt;
Signed-off-by: Eelco Chaudron &lt;echaudro@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
</pre>
</div>
</content>
</entry>
</feed>
