[ovs-dev] Interface not coming up after node reboot /restart !!
Hi folks,
We are running into a issue on node reboot and on restart of network
manager. Post this activity sometime (NOT always) some of the interfaces do
not come up with configured ip address The configuration on OVS looks like
below
Bridge br_data
Port ecfe_cran
tag: 810
Interface ecfe_cran
type: internal
Port ccd_int
tag: 3220
Interface ccd_int
type: internal
Port ccd_backhaul
tag: 822
Interface ccd_backhaul
type: internal
Port ccd_midhaul
tag: 821
Interface ccd_midhaul
type: internal
Port ccd_ecfe_om
tag: 805
Interface ccd_ecfe_om
type: internal
Port ccd_hs_san
tag: 3609
Interface ccd_hs_san
type: internal
Port ccd_hs_repl
tag: 3608
Interface ccd_hs_repl
type: internal
Port ccd_om
tag: 801
Interface ccd_om
type: internal
Port bond_data
Interface bond_data
type: system
ovs_version: "2.14.2"
On further analysis , it seems that the even though network manager(NM)
sends a delete and add of the problematic interface to the OVSDB server ,
the vswitch process does not delete and add it .
2022-09-06T13:01:38.102Z|29503|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
send request, method="transact",
params=["Open_vSwitch",{"lock":"ovs_vswitchd","op":"assert"},{"where":[["_uu
id","==",["uuid","4177a4b7-8d05-4f3d-a5c3-7c6aaf9d4cb6"]]],"row":{"cur_cfg":
4614},"op":"update","table":"Open_vSwitch"}], id=1552
2022-09-06T13:01:38.107Z|29506|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received notification, method="update3",
params=[["monid","Open_vSwitch"],"----",{"In
terface":{"a014bb2a-5c26-47e9-9e29-177d299fbf2e":{"delete":null}},"Port":{"4
3b5c9b6-18ca-4a29-997a-2adf2cbb92ac":{"delete":null}},"Bridge":{"f066df83-6e
85-4120-8c6c-7aecb493953d":{"delete":null}},"Open_vSwitch":{"4177a4b7-8d05-4
f3d-a5c3-7c6aaf9d4cb6":{"modify":{"bridges":["uuid","f066df83-6e85-4120-8c6c
-7aecb493953d"],"next_cfg":4615]
2022-09-06T13:01:38.107Z|29507|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received notification, method="update3",
params=[["monid","Open_vSwitch"],"----",{"In
terface":{"742c022b-e8b7-45d1-a738-a3c0107ee852":{"insert":{"name":"bond_dat
a","type":"system"}}},"Port":{"f39baeca-5d84-4716-bbc4-65b1fa574cf2":{"inser
t":{"name":"bond_data","interfaces":["uuid","742c022b-e8b7-45d1-a738-a3c0107
ee852"]}}},"Bridge":{"6e5e7d3d-582a-4a34-9b73-d5558ea4f019":{"insert":{"name
":"br_data","ports":["uuid","f39baeca-5d84-4716-bbc4-65b1fa574cf2"]}}},"Open
_vSwitch":{"4177a4b7-8d05-4f3d-a5c3-7c6aaf9d4cb6":{"modify":{"bridges":["uui
d","6e5e7d3d-582a-4a34-9b73-d5558ea4f019"],"next_cfg":4616]
2022-09-06T13:01:38.107Z|29508|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received notification, method="update3",
params=[["monid","Open_vSwitch"],"----",{"In
terface":{"343c7159-92f1-4eed-9925-3b5369185ae7":{"insert":{"name":"ccd_ecfe
_om","mtu_request":1500,"type":"internal"}}},"Port":{"d1de4f3d-c275-4d7d-8d3
c-69eb70f91618":{"insert":{"name":"ccd_ecfe_om","interfaces":["uuid","343c71
59-92f1-4eed-9925-3b5369185ae7"],"vlan_mode":"access","tag":805}}},"Bridge":
{"6e5e7d3d-582a-4a34-9b73-d5558ea4f019":{"modify":{"ports":["uuid","d1de4f3d
-c275-4d7d-8d3c-69eb70f91618"]}}},"Open_vSwitch":{"4177a4b7-8d05-4f3d-a5c3-7
c6aaf9d4cb6":{"modify":{"next_cfg":4617]
2022-09-06T13:01:38.107Z|29509|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received notification, method="update3",
params=[["monid","Open_vSwitch"],"----",{"In
terface":{"0dc33eae-1dab-4767-a43d-493e050e5bf2":{"insert":{"name":"ccd_hs_r
epl","mtu_request":9000,"type":"internal"}}},"Port":{"3ea58cf4-cac4-4d65-9e0
4-b9279ed2b68c":{"insert":{"name":"ccd_hs_repl","interfaces":["uuid","0dc33e
ae-1dab-4767-a43d-493e050e5bf2"],"vlan_mode":"access","tag":3608}}},"Bridge"
:{"6e5e7d3d-582a-4a34-9b73-d5558ea4f019":{"modify":{"ports":["uuid","3ea58cf
4-cac4-4d65-9e04-b9279ed2b68c"]}}},"Open_vSwitch":{"4177a4b7-8d05-4f3d-a5c3-
7c6aaf9d4cb6":{"modify":{"next_cfg":4618]
2022-09-06T13:01:38.107Z|29510|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received notification, method="update3",
params=[["monid","Open_vSwitch"],"----",{"In
terface":{"bba298eb-0796-41a1-9793-35abc68e67df":{"insert":{"name":"ccd_hs_s
an","mtu_request":9000,"type":"internal"}}},"Port":{"52526bd5-ee2c-4c7c-b4a3
-b2e364f4782f":{"insert":{"name":"ccd_hs_san","interfaces":["uuid","bba298eb
-0796-41a1-9793-35abc68e67df"],"vlan_mode":"access","tag":3609}}},"Bridge":{
"6e5e7d3d-582a-4a34-9b73-
Re: [ovs-dev] [PATCH v18] Improved Packet Drop Statistics in OVS
Thank you Ilya & Ben . Regards Anju -Original Message- From: Ilya Maximets Sent: Tuesday, January 7, 2020 10:51 PM To: Ben Pfaff ; Ilya Maximets Cc: Anju Thomas ; d...@openvswitch.org; echau...@redhat.com; Rohith Basavaraja ; Keshav Gupta Subject: Re: [PATCH v18] Improved Packet Drop Statistics in OVS On 06.01.2020 22:21, Ben Pfaff wrote: > On Wed, Dec 18, 2019 at 03:20:30PM +0100, Ilya Maximets wrote: >> Hi Ben, >> could you, please, take a look at this patch one more time? >> There were a couple of changes as we moved 'enum xlate_error' to >> openvswitch.h header to avoid inclusion of 'ofproto/ofproto-dpif-xlate.h' >> from the 'lib' code, new datapath capability was documented and we >> cleaned the patch up a little bit. > > I confess that I missed this request until today when someone pointed > it out. > > Let's get this in. > > Acked-by: Ben Pfaff > Thanks! Applied to master. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v18] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing
are counted in flow stats of “drop” flows or as table misses in table
stats. These can only be interpreted by controllers that know the
semantics of the configured OpenFlow pipeline. Without that knowledge,
it is impossible for an OVS user to obtain e.g. the total number of
packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert a
management system in case of a unexpected increase of such drops.
AlsoOVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
Acked-by: Eelco Chaudron size
3. Add version history
v12(Ben's comments)
1. new dp action in kernel block in openvswitch.h
2. change xlate_error enum to be used as u32
3. resetting xlate error in case of congestion drop
and forwarding disabled
Signed-off-by: Anju Thomas
---
datapath/linux/compat/include/linux/openvswitch.h | 24 +++
lib/dpif-netdev.c | 44 -
lib/dpif.c| 7 +
lib/dpif.h| 1 +
lib/odp-execute.c | 78 +
lib/odp-util.c| 5 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 33 +++-
ofproto/ofproto-dpif-xlate.h | 13 --
ofproto/ofproto-dpif.c| 10 ++
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 190 ++
tests/ofproto-dpif.at | 2 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 +++-
tests/tunnel.at | 16 +-
vswitchd/vswitch.xml | 5 +
20 files changed, 454 insertions(+), 24 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 778827f..fef1bc8 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -407,6 +407,28 @@ enum ovs_tunnel_key_attr {
#define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1)
/**
+ * enum xlate_error - Different types of error during translation
+ */
+
+#ifndef __KERNEL__
+enum xlate_error {
+ XLATE_OK = 0,
+ XLATE_BRIDGE_NOT_FOUND,
+ XLATE_RECURSION_TOO_DEEP,
+ XLATE_TOO_MANY_RESUBMITS,
+ XLATE_STACK_TOO_DEEP,
+ XLATE_NO_RECIRCULATION_CONTEXT,
+ XLATE_RECIRCULATION_CONFLICT,
+ XLATE_TOO_MANY_MPLS_LABELS,
+ XLATE_INVALID_TUNNEL_METADATA,
+ XLATE_UNSUPPORTED_PACKET_TYPE,
+ XLATE_CONGESTION_DROP,
+ XLATE_FORWARDING_DISABLED,
+ XLATE_MAX,
+};
+#endif
+
+/**
* enum ovs_frag_type - IPv4 and IPv6 fragment type
* @OVS_FRAG_TYPE_NONE: Packet is not a fragment.
* @OVS_FRAG_TYPE_FIRST: Packet is a fragment with offset 0.
@@ -962,6 +984,7 @@ struct check_pkt_len_arg {
* @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
* of actions if greater than the specified packet length, else execute
* another set of actions.
+ * @OVS_ACTION_ATTR_DROP: Explicit drop action.
*/
enum ovs_action_attr {
@@ -994,6 +1017,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3f21211..f7
Re: [ovs-dev] [PATCH v17] Improved Packet Drop Statistics in OVS
Thanks for the review Ilya, For the below comment > @@ -7086,10 +7105,16 @@ dp_execute_cb(void *aux_, struct dp_packet_batch > *packets_, > * the ownership of these packets. Thus, we can avoid performing > * the action, because the caller will not use the result anyway. > * Just break to free the batch. */ > +COVERAGE_ADD(datapath_drop_tunnel_push_error, > + dp_packet_batch_size(packets_)); This is not a tunnel push error. We literally executed all the actions without errors and that is not our fault that there are no more actions after tnl_push. We're just saving some time avoiding actual execution of a pointless action. So, this should not be accounted as error, and definitely not as a tunnel push error. I agree we need a better name .Does datapath_drop_incomplete_dp_action sound good or do you have any other suggestions in mind? Regards & Thanks Anju T -Original Message- From: Ilya Maximets Sent: Friday, December 13, 2019 9:57 PM To: Anju Thomas ; d...@openvswitch.org Cc: i.maxim...@ovn.org; echau...@redhat.com; Rohith Basavaraja ; Keshav Gupta Subject: Re: [PATCH v17] Improved Packet Drop Statistics in OVS On 06.12.2019 09:49, Anju Thomas wrote: > Currently OVS maintains explicit packet drop/error counters only on > port level. Packets that are dropped as part of normal OpenFlow > processing are counted in flow stats of “drop” flows or as table > misses in table stats. These can only be interpreted by controllers > that know the semantics of the configured OpenFlow pipeline. Without > that knowledge, it is impossible for an OVS user to obtain e.g. the > total number of packets dropped due to OpenFlow rules. > > Furthermore, there are numerous other reasons for which packets can be > dropped by OVS slow path that are not related to the OpenFlow pipeline. > The generated datapath flow entries include a drop action to avoid > further expensive upcalls to the slow path, but subsequent packets > dropped by the datapath are not accounted anywhere. > > Finally, the datapath itself drops packets in certain error situations. > Also, these drops are today not accounted for.This makes it difficult > for OVS users to monitor packet drop in an OVS instance and to alert a > management system in case of a unexpected increase of such drops. > Also OVS trouble-shooters face difficulties in analysing packet drops. > > With this patch we implement following changes to address the issues > mentioned above. > > 1. Identify and account all the silent packet drop scenarios > > 2. Display these drops in ovs-appctl coverage/show > > Co-authored-by: Rohith Basavaraja > Co-authored-by: Keshav Gupta > Signed-off-by: Anju Thomas > Signed-off-by: Rohith Basavaraja > Signed-off-by: Keshav Gupta > Acked-by: Eelco Chaudron --- Thanks for a new version. I really hope that v18 will be the last one. This patch needs some minor rebase on top of current master. 6 more comments inline (some of them are really minor, but I decided to point to them as you'll re-spin the patch anyway). Please, fix them and send v18. Best regards, Ilya Maximets. > v17 (Ilya's comments) > 1. comments for xlate_error > 2. define xlate_error in ifndef __KERNEL__ 3. move declaration of > xlate_error after OVS_TUNNEL_KEY_ATTR_MAX 4. comment change for DROP > action 5. Add in datapath capability > > v16 (Ilya and Eelco comments) > 1. remove old declaration of xlate_error in v15 2. kernel file > formatting in openvswitch.h for xlate_error 3. remove drop_action > from odp_actions_from_string > > v15(Ilya's comments) > 1. Description in openvswitch.h > 2. Move xlate_error to openvswitch.h to not include ofproto heeader > in dp 3. Return u32 for instead of size of enum 4. Formatting 5. > Add drop-stats in testsuite.at 6. Use coverage-read-counter 7. Use > flow instead of raw packet data wherever possible 8. Change sleep to > warp > > v14 (Eelco's comments) > 1. Remove definition of dpif_show_drop_stats_support 2. Remove extra > check for drop reason in odp_execute_actions > > v13(Eelco and Illya's comments) > 1. packet_dropped changed to packets_dropped 2. Uses > dp_packet_batch_size instead of packet->size 3. Add version history > > v12(Ben's comments) > 1. new dp action in kernel block in openvswitch.h 2. change > xlate_error enum to be used as u32 3. resetting xlate error in case > of congestion drop > and forwarding disabled > --- > datapath/linux/compat/include/linux/openvswitch.h | 24 +++ > lib/dpif-netdev.c | 47 +- > lib/dpif.c| 7 +
[ovs-dev] [PATCH v17] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing
are counted in flow stats of “drop” flows or as table misses in table
stats. These can only be interpreted by controllers that know the
semantics of the configured OpenFlow pipeline. Without that knowledge,
it is impossible for an OVS user to obtain e.g. the total number of
packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert a
management system in case of a unexpected increase of such drops. Also
OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
Acked-by: Eelco Chaudron size
3. Add version history
v12(Ben's comments)
1. new dp action in kernel block in openvswitch.h
2. change xlate_error enum to be used as u32
3. resetting xlate error in case of congestion drop
and forwarding disabled
---
datapath/linux/compat/include/linux/openvswitch.h | 24 +++
lib/dpif-netdev.c | 47 +-
lib/dpif.c| 7 +
lib/dpif.h| 2 +
lib/odp-execute.c | 77 +
lib/odp-util.c| 5 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 33 +++-
ofproto/ofproto-dpif-xlate.h | 13 --
ofproto/ofproto-dpif.c| 10 ++
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 190 ++
tests/ofproto-dpif.at | 2 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 +++-
tests/tunnel.at | 16 +-
vswitchd/vswitch.xml | 5 +
20 files changed, 457 insertions(+), 24 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 778827f..8006724 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -407,6 +407,28 @@ enum ovs_tunnel_key_attr {
#define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1)
/**
+ * enum xlate_error - Different types of error during translation
+ */
+
+#ifndef __KERNEL__
+enum xlate_error {
+XLATE_OK = 0,
+XLATE_BRIDGE_NOT_FOUND,
+XLATE_RECURSION_TOO_DEEP,
+XLATE_TOO_MANY_RESUBMITS,
+XLATE_STACK_TOO_DEEP,
+XLATE_NO_RECIRCULATION_CONTEXT,
+XLATE_RECIRCULATION_CONFLICT,
+XLATE_TOO_MANY_MPLS_LABELS,
+XLATE_INVALID_TUNNEL_METADATA,
+XLATE_UNSUPPORTED_PACKET_TYPE,
+XLATE_CONGESTION_DROP,
+XLATE_FORWARDING_DISABLED,
+XLATE_MAX,
+};
+#endif
+
+/**
* enum ovs_frag_type - IPv4 and IPv6 fragment type
* @OVS_FRAG_TYPE_NONE: Packet is not a fragment.
* @OVS_FRAG_TYPE_FIRST: Packet is a fragment with offset 0.
@@ -962,6 +984,7 @@ struct check_pkt_len_arg {
* @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
* of actions if greater than the specified packet length, else execute
* another set of actions.
+ * @OVS_ACTION_ATTR_DROP: Explicit drop action.
*/
enum ovs_action_attr {
@@ -994,6 +1017,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP, /* u32 xlate_error. */
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5142bad..8adeac5 100644
---
Re: [ovs-dev] [PATCH v16] Improved Packet Drop Statistics in OVS
Hi Illya, I am unclear regarding one comment from your end . You have suggested to put the definition of xlate_error in ifndef __KERNEL__. But these errors are populated in ofproto layer even otherwise as well. Regards & Thanks Anju -Original Message- From: Ilya Maximets Sent: Wednesday, November 27, 2019 1:37 AM To: Ilya Maximets ; Anju Thomas ; d...@openvswitch.org Cc: echau...@redhat.com; Rohith Basavaraja ; Keshav Gupta Subject: Re: [PATCH v16] Improved Packet Drop Statistics in OVS On 26.11.2019 20:43, Ilya Maximets wrote: > On 26.11.2019 9:59, Anju Thomas wrote: >> Currently OVS maintains explicit packet drop/error counters only on port >> level. Packets that are dropped as part of normal OpenFlow processing are >> counted in flow stats of “drop” flows or as table misses in table stats. >> These can only be interpreted by controllers that know the semantics of the >> configured OpenFlow pipeline. >> Without that knowledge, it is impossible for an OVS user to obtain e.g. the >> total number of packets dropped due to OpenFlow rules. >> >> Furthermore, there are numerous other reasons for which packets can be >> dropped by OVS slow path that are not related to the OpenFlow pipeline. >> The generated datapath flow entries include a drop action to avoid further >> expensive upcalls to the slow path, but subsequent packets dropped by the >> datapath are not accounted anywhere. >> >> Finally, the datapath itself drops packets in certain error situations. >> Also, these drops are today not accounted for.This makes it difficult for >> OVS users to monitor packet drop in an OVS instance and to alert a >> management system in case of a unexpected increase of such drops. >> Also OVS trouble-shooters face difficulties in analysing packet drops. >> >> With this patch we implement following changes to address the issues >> mentioned above. >> >> 1. Identify and account all the silent packet drop scenarios >> >> 2. Display these drops in ovs-appctl coverage/show >> >> Co-authored-by: Rohith Basavaraja >> Co-authored-by: Keshav Gupta >> Signed-off-by: Anju Thomas >> Signed-off-by: Rohith Basavaraja >> Signed-off-by: Keshav Gupta >> Acked-by: Eelco Chaudron > Please, limit the line length in a commit message to 72 characters. > >> --- >> v16 (Ilya and Eelco comments) >> 1. remove old declaration of xlate_error in v15 2. kernel file >> formatting in openvswitch.h for xlate_error 3. remove drop_action >> from odp_actions_from_string >> >> v15(Ilya's comments) >> 1. Description in openvswitch.h >> 2. Move xlate_error to openvswitch.h to not include ofproto heeader >> in dp 3. Return u32 for instead of size of enum 4. Formatting 5. >> Add drop-stats in testsuite.at 6. Use coverage-read-counter 7. Use >> flow instead of raw packet data wherever possible 8. Change sleep to >> warp >> >> v14 (Eelco's comments) >> 1. Remove definition of dpif_show_drop_stats_support 2. Remove >> extra check for drop reason in odp_execute_actions >> >> v13(Eelco and Illya's comments) >> 1. packet_dropped changed to packets_dropped 2. Uses >> dp_packet_batch_size instead of packet->size 3. Add version history >> >> v12(Ben's comments) >> 1. new dp action in kernel block in openvswitch.h 2. change >> xlate_error enum to be used as u32 3. resetting xlate error in case of >> congestion drop >> and forwarding disabled >> --- >> datapath/linux/compat/include/linux/openvswitch.h | 18 ++ >> lib/dpif-netdev.c | 47 +- >> lib/dpif.c| 7 + >> lib/dpif.h| 2 + >> lib/odp-execute.c | 77 + >> lib/odp-util.c| 5 + >> ofproto/ofproto-dpif-ipfix.c | 1 + >> ofproto/ofproto-dpif-sflow.c | 1 + >> ofproto/ofproto-dpif-xlate.c | 38 - >> ofproto/ofproto-dpif-xlate.h | 13 -- >> ofproto/ofproto-dpif.c| 8 + >> ofproto/ofproto-dpif.h| 8 +- >> tests/automake.mk | 3 +- >> tests/dpif-netdev.at | 8 + >> tests/drop-stats.at | 190 >> ++ >> tests/ofproto-dpif.at | 2 +- >> tests/testsuite.at
[ovs-dev] [PATCH v16] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level.
Packets that are dropped as part of normal OpenFlow processing are counted in
flow stats of “drop” flows or as table misses in table stats. These can only be
interpreted by controllers that know the semantics of the configured OpenFlow
pipeline.
Without that knowledge, it is impossible for an OVS user to obtain e.g. the
total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be dropped
by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult for OVS
users to monitor packet drop in an OVS instance and to alert a management
system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues mentioned
above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
Acked-by: Eelco Chaudron size
3. Add version history
v12(Ben's comments)
1. new dp action in kernel block in openvswitch.h
2. change xlate_error enum to be used as u32 3. resetting xlate error in case
of congestion drop
and forwarding disabled
---
datapath/linux/compat/include/linux/openvswitch.h | 18 ++
lib/dpif-netdev.c | 47 +-
lib/dpif.c| 7 +
lib/dpif.h| 2 +
lib/odp-execute.c | 77 +
lib/odp-util.c| 5 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 38 -
ofproto/ofproto-dpif-xlate.h | 13 --
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 190 ++
tests/ofproto-dpif.at | 2 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 +++-
tests/tunnel.at | 16 +-
19 files changed, 449 insertions(+), 24 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 778827f..e974d2c 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -404,6 +404,22 @@ enum ovs_tunnel_key_attr {
__OVS_TUNNEL_KEY_ATTR_MAX
};
+enum xlate_error {
+ XLATE_OK = 0,
+ XLATE_BRIDGE_NOT_FOUND,
+ XLATE_RECURSION_TOO_DEEP,
+ XLATE_TOO_MANY_RESUBMITS,
+ XLATE_STACK_TOO_DEEP,
+ XLATE_NO_RECIRCULATION_CONTEXT,
+ XLATE_RECIRCULATION_CONFLICT,
+ XLATE_TOO_MANY_MPLS_LABELS,
+ XLATE_INVALID_TUNNEL_METADATA,
+ XLATE_UNSUPPORTED_PACKET_TYPE,
+ XLATE_CONGESTION_DROP,
+ XLATE_FORWARDING_DISABLED,
+ XLATE_MAX,
+};
+
#define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1)
/**
@@ -962,6 +978,7 @@ struct check_pkt_len_arg {
* @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
* of actions if greater than the specified packet length, else execute
* another set of actions.
+ * @OVS_ACTION_ATTR_DROP: Explicit drop action.
*/
enum ovs_action_attr {
@@ -994,6 +1011,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP, /* explicit drop action. */
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5142bad..8adeac5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,6 +102,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_D
[ovs-dev] [PATCH v15] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level.
Packets that are dropped as part of normal OpenFlow processing are counted in
flow stats of “drop” flows or as table misses in table stats. These can only be
interpreted by controllers that know the semantics of the configured OpenFlow
pipeline.
Without that knowledge, it is impossible for an OVS user to obtain e.g. the
total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be dropped
by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult for OVS
users to monitor packet drop in an OVS instance and to alert a management
system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues mentioned
above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
Acked-by: Eelco Chaudron size
3. Add version history
v12(Ben's comments)
1. new dp action in kernel block in openvswitch.h
2. change xlate_error enum to be used as u32 3. resetting xlate error in case
of congestion drop
and forwarding disabled
---
datapath/linux/compat/include/linux/openvswitch.h | 18 ++
lib/dpif-netdev.c | 47 +-
lib/dpif.c| 7 +
lib/dpif.h| 2 +
lib/odp-execute.c | 78 +
lib/odp-util.c| 7 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 38 -
ofproto/ofproto-dpif-xlate.h | 7 +-
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 190 ++
tests/ofproto-dpif.at | 2 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 +++-
tests/tunnel.at | 16 +-
19 files changed, 457 insertions(+), 13 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 7b16b1d..60525a2 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -403,6 +403,22 @@ enum ovs_tunnel_key_attr {
__OVS_TUNNEL_KEY_ATTR_MAX
};
+enum xlate_error {
+XLATE_OK = 0,
+XLATE_BRIDGE_NOT_FOUND,
+XLATE_RECURSION_TOO_DEEP,
+XLATE_TOO_MANY_RESUBMITS,
+XLATE_STACK_TOO_DEEP,
+XLATE_NO_RECIRCULATION_CONTEXT,
+XLATE_RECIRCULATION_CONFLICT,
+XLATE_TOO_MANY_MPLS_LABELS,
+XLATE_INVALID_TUNNEL_METADATA,
+XLATE_UNSUPPORTED_PACKET_TYPE,
+XLATE_CONGESTION_DROP,
+XLATE_FORWARDING_DISABLED,
+XLATE_MAX,
+};
+
#define OVS_TUNNEL_KEY_ATTR_MAX (__OVS_TUNNEL_KEY_ATTR_MAX - 1)
/**
@@ -961,6 +977,7 @@ struct check_pkt_len_arg {
* @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set
* of actions if greater than the specified packet length, else execute
* another set of actions.
+ * @OVS_ACTION_ATTR_DROP: Explicit drop action.
*/
enum ovs_action_attr {
@@ -993,6 +1010,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP, /* explicit drop action. */
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4720ba1..8835158 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,6 +102,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_D
Re: [ovs-dev] [PATCH v14] Improved Packet Drop Statistics in OVS
Thanks for the review Eelco. Regards Anju -Original Message- From: Eelco Chaudron Sent: Tuesday, November 5, 2019 1:23 PM To: Anju Thomas Cc: d...@openvswitch.org; i.maxim...@samsung.com; b...@ovn.org; Rohith Basavaraja ; Keshav Gupta Subject: Re: [PATCH v14] Improved Packet Drop Statistics in OVS On 5 Nov 2019, at 5:35, Anju Thomas wrote: > Currently OVS maintains explicit packet drop/error counters only on > port level. Packets that are dropped as part of normal OpenFlow > processing are counted in flow stats of “drop” flows or as table > misses in table stats. These can only be interpreted by controllers > that know the semantics of the configured OpenFlow pipeline. > Without that knowledge, it is impossible for an OVS user to obtain > e.g. the total number of packets dropped due to OpenFlow rules. > > Furthermore, there are numerous other reasons for which packets can be > dropped by OVS slow path that are not related to the OpenFlow > pipeline. > The generated datapath flow entries include a drop action to avoid > further expensive upcalls to the slow path, but subsequent packets > dropped by the datapath are not accounted anywhere. > > Finally, the datapath itself drops packets in certain error > situations. > Also, these drops are today not accounted for.This makes it difficult > for OVS users to monitor packet drop in an OVS instance and to alert a > management system in case of a unexpected increase of such drops. > Also OVS trouble-shooters face difficulties in analysing packet drops. > > With this patch we implement following changes to address the issues > mentioned above. > > 1. Identify and account all the silent packet drop scenarios > > 2. Display these drops in ovs-appctl coverage/show > > Co-authored-by: Rohith Basavaraja > Co-authored-by: Keshav Gupta > Signed-off-by: Anju Thomas > Signed-off-by: Rohith Basavaraja > Signed-off-by: Keshav Gupta Thanks for following this trough! Did some quick sanity tests and based on my previous review of v13 and the diff to v14 it looks good to me! Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v14] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level.
Packets that are dropped as part of normal OpenFlow processing are counted in
flow stats of “drop” flows or as table misses in table stats. These can only be
interpreted by controllers that know the semantics of the configured OpenFlow
pipeline.
Without that knowledge, it is impossible for an OVS user to obtain e.g. the
total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be dropped
by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult for OVS
users to monitor packet drop in an OVS instance and to alert a management
system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues mentioned
above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
v14 (Eelco's comments)
1. Remove definition of dpif_show_drop_stats_support
2. Remove extra check for drop reason in odp_execute_actions
v13(Eelco and Illya's comments)
1. packet_dropped changed to packets_dropped 2. Uses dp_packet_batch_size
instead of packet->size 3. Add version history
v12(Ben's comments)
1. new dp action in kernel block in openvswitch.h 2. change xlate_error enum to
be used as u32 3. resetting xlate error in case of congestion drop
and forwarding disabled
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 47 -
lib/dpif.c| 7 +
lib/dpif.h| 2 +
lib/odp-execute.c | 78
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 40 -
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 210 ++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 16 +-
18 files changed, 462 insertions(+), 11 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 7b16b1d..798b5aa 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -993,6 +993,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP,
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4720ba1..8835158 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,6 +102,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5749,7 +5760,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band = &meter->bands[exceeded_band[j]];
band->packet_cou
Re: [ovs-dev] [PATCH v13] Improved Packet Drop Statistics in OVS
Hi Eelco, I have not got any more comments . So I will send out v14 soon. Hopefully there are no more changes 😊 Regards Anju -Original Message- From: Eelco Chaudron Sent: Thursday, October 31, 2019 6:08 PM To: Anju Thomas Cc: d...@openvswitch.org; i.maxim...@samsung.com; Keshav Gupta Subject: Re: [ovs-dev] [PATCH v13] Improved Packet Drop Statistics in OVS Got no response, so I’m trying again, see below :) On 18 Oct 2019, at 13:50, Eelco Chaudron wrote: > Are you planning on sending a v14? > > Cheers, > > Eelco > > > On 16 Sep 2019, at 16:12, Eelco Chaudron wrote: > >> Hi Anju, >> >> For some reason, this version does not show up in patchwork! I have >> two small comments below but other than that it looks fine to me. >> >> If those two will be the only changes in your next rev, I’ll ack it >> straight away :) >> >> Cheers, >> >> Eelco >> >> On 9 Sep 2019, at 13:53, Anju Thomas wrote: >> >>> Currently OVS maintains explicit packet drop/error counters only on >>> port level. Packets that are dropped as part of normal OpenFlow >>> processing are counted in flow stats of “drop” flows or as table >>> misses in table stats. These can only be interpreted by controllers >>> that know the semantics of the configured OpenFlow pipeline. >>> Without that knowledge, it is impossible for an OVS user to obtain >>> e.g. the total number of packets dropped due to OpenFlow rules. >>> >>> Furthermore, there are numerous other reasons for which packets can >>> be dropped by OVS slow path that are not related to the OpenFlow >>> pipeline. >>> The generated datapath flow entries include a drop action to avoid >>> further expensive upcalls to the slow path, but subsequent packets >>> dropped by the datapath are not accounted anywhere. >>> >>> Finally, the datapath itself drops packets in certain error >>> situations. >>> Also, these drops are today not accounted for.This makes it >>> difficult for OVS users to monitor packet drop in an OVS instance >>> and to alert a management system in case of a unexpected increase of >>> such drops. >>> Also OVS trouble-shooters face difficulties in analysing packet >>> drops. >>> >>> With this patch we implement following changes to address the issues >>> mentioned above. >>> >>> 1. Identify and account all the silent packet drop scenarios >>> >>> 2. Display these drops in ovs-appctl coverage/show >>> >>> Co-authored-by: Rohith Basavaraja >>> Co-authored-by: Keshav Gupta >>> Signed-off-by: Anju Thomas >>> Signed-off-by: Rohith Basavaraja >>> Signed-off-by: Keshav Gupta >>> --- >>> v13(Eelco and Illya's comments) >>> 1. packet_dropped changed to packets_dropped 2. Uses >>> dp_packet_batch_size instead of packet->size 3. Add version history >>> >>> v12(Ben's comments) >>> 1. new dp action in kernel block in openvswitch.h 2. change >>> xlate_error enum to be used as u32 3. resetting xlate error in case >>> of congestion drop >>>and forwarding disabled >>> >>> datapath/linux/compat/include/linux/openvswitch.h | 1 + >>> lib/dpif-netdev.c | 47 - >>> lib/dpif.c| 7 + >>> lib/dpif.h| 4 + >>> lib/odp-execute.c | 79 >>> lib/odp-util.c| 9 + >>> ofproto/ofproto-dpif-ipfix.c | 1 + >>> ofproto/ofproto-dpif-sflow.c | 1 + >>> ofproto/ofproto-dpif-xlate.c | 40 - >>> ofproto/ofproto-dpif-xlate.h | 3 + >>> ofproto/ofproto-dpif.c| 8 + >>> ofproto/ofproto-dpif.h| 8 +- >>> tests/automake.mk | 3 +- >>> tests/dpif-netdev.at | 8 + >>> tests/drop-stats.at | 210 >>> ++ >>> tests/ofproto-dpif.at | 2 +- >>> tests/tunnel-push-pop.at | 29 ++- >>> tests/tunnel.at | 16 +- >>> 18 files changed, 465 insertions(+), 11 deletions(-) create mode >>> 100644 tests/drop-stats.at >&
[ovs-dev] [PATCH v13] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on
port level. Packets that are dropped as part of normal OpenFlow
processing are counted in flow stats of “drop” flows or as table
misses in table stats. These can only be interpreted by controllers
that know the semantics of the configured OpenFlow pipeline.
Without that knowledge, it is impossible for an OVS user to obtain
e.g. the total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can
be dropped by OVS slow path that are not related to the OpenFlow
pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert
a management system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
v13(Eelco and Illya's comments)
1. packet_dropped changed to packets_dropped
2. Uses dp_packet_batch_size instead of packet->size
3. Add version history
v12(Ben's comments)
1. new dp action in kernel block in openvswitch.h
2. change xlate_error enum to be used as u32
3. resetting xlate error in case of congestion drop
and forwarding disabled
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 47 -
lib/dpif.c| 7 +
lib/dpif.h| 4 +
lib/odp-execute.c | 79
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 40 -
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 210 ++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 16 +-
18 files changed, 465 insertions(+), 11 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..415c053 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -989,6 +989,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP,
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a88a78f..8f811be 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,6 +102,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5741,7 +5752,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band = &meter->bands[exceeded_band[j]];
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
-
+COVERAGE_INC(datapath_drop_meter);
dp_pa
Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS
Hi Eelco,
Since I have not received any more comments, let me respond to your comments:
-Original Message-
From: Eelco Chaudron
Sent: Tuesday, September 3, 2019 7:27 PM
To: Anju Thomas
Cc: d...@openvswitch.org; Keshav Gupta
Subject: Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS
On 20 Aug 2019, at 11:57, Anju Thomas wrote:
> Thanks for the comments Eelco. I will address them in the next patch.
>
> On a similar note, are these the final set of comments for this patch
> ?
Did you get any update, and are you planning on sending out an update?
Thanks,
Eelco
> Regards
> Anju
>
> -Original Message-
> From: Eelco Chaudron
> Sent: Monday, August 12, 2019 5:48 PM
> To: Anju Thomas
> Cc: d...@openvswitch.org; Keshav Gupta
> Subject: Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in
> OVS
>
> Hi Anju,
>
> See comments inline…
>
> //Eelco
>
>
> On 25 Jul 2019, at 14:16, Anju Thomas wrote:
>
>> Currently OVS maintains explicit packet drop/error counters only on
>> port level. Packets that are dropped as part of normal OpenFlow
>> processing are counted in flow stats of “drop” flows or as table
>> misses in table stats. These can only be interpreted by controllers
>> that know the semantics of the configured OpenFlow pipeline.
>> Without that knowledge, it is impossible for an OVS user to obtain
>> e.g. the total number of packets dropped due to OpenFlow rules.
>>
>> Furthermore, there are numerous other reasons for which packets can
>> be dropped by OVS slow path that are not related to the OpenFlow
>> pipeline.
>> The generated datapath flow entries include a drop action to avoid
>> further expensive upcalls to the slow path, but subsequent packets
>> dropped by the datapath are not accounted anywhere.
>>
>> Finally, the datapath itself drops packets in certain error
>> situations.
>> Also, these drops are today not accounted for.This makes it difficult
>> for OVS users to monitor packet drop in an OVS instance and to alert
>> a management system in case of a unexpected increase of such drops.
>> Also OVS trouble-shooters face difficulties in analysing packet
>> drops.
>>
>> With this patch we implement following changes to address the issues
>> mentioned above.
>>
>> 1. Identify and account all the silent packet drop scenarios
>>
>> 2. Display these drops in ovs-appctl coverage/show
>>
>> v11->v12 Addresses comments from Ben Pfaff
>>
>> Co-authored-by: Rohith Basavaraja
>> Co-authored-by: Keshav Gupta
>> Signed-off-by: Anju Thomas
>> Signed-off-by: Rohith Basavaraja
>> Signed-off-by: Keshav Gupta
>> ---
>> datapath/linux/compat/include/linux/openvswitch.h | 1 +
>> lib/dpif-netdev.c | 46 -
>> lib/dpif.c| 7 +
>> lib/dpif.h| 4 +
>> lib/odp-execute.c | 79
>> lib/odp-util.c| 9 +
>> ofproto/ofproto-dpif-ipfix.c | 1 +
>> ofproto/ofproto-dpif-sflow.c | 1 +
>> ofproto/ofproto-dpif-xlate.c | 40 -
>> ofproto/ofproto-dpif-xlate.h | 3 +
>> ofproto/ofproto-dpif.c| 8 +
>> ofproto/ofproto-dpif.h| 8 +-
>> tests/automake.mk | 3 +-
>> tests/dpif-netdev.at | 8 +
>> tests/drop-stats.at | 210
>> ++
>> tests/ofproto-dpif.at | 2 +-
>> tests/tunnel-push-pop.at | 29 ++-
>> tests/tunnel.at | 16 +-
>> 18 files changed, 464 insertions(+), 11 deletions(-) create mode
>> 100644 tests/drop-stats.at
>>
>> diff --git a/datapath/linux/compat/include/linux/openvswitch.h
>> b/datapath/linux/compat/include/linux/openvswitch.h
>> index 65a003a..415c053 100644
>> --- a/datapath/linux/compat/include/linux/openvswitch.h
>> +++ b/datapath/linux/compat/include/linux/openvswitch.h
>> @@ -989,6 +989,7 @@ enum ovs_action_attr { #ifndef __KERNEL__
>> OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
>> OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
>> +OVS_ACTION_ATTR_DROP,
>> #endif
>> __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
>&g
Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS
Thanks for the comments Eelco. I will address them in the next patch.
On a similar note, are these the final set of comments for this patch ?
Regards
Anju
-Original Message-
From: Eelco Chaudron
Sent: Monday, August 12, 2019 5:48 PM
To: Anju Thomas
Cc: d...@openvswitch.org; Keshav Gupta
Subject: Re: [ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS
Hi Anju,
See comments inline…
//Eelco
On 25 Jul 2019, at 14:16, Anju Thomas wrote:
> Currently OVS maintains explicit packet drop/error counters only on
> port level. Packets that are dropped as part of normal OpenFlow
> processing are counted in flow stats of “drop” flows or as table
> misses in table stats. These can only be interpreted by controllers
> that know the semantics of the configured OpenFlow pipeline.
> Without that knowledge, it is impossible for an OVS user to obtain
> e.g. the total number of packets dropped due to OpenFlow rules.
>
> Furthermore, there are numerous other reasons for which packets can be
> dropped by OVS slow path that are not related to the OpenFlow
> pipeline.
> The generated datapath flow entries include a drop action to avoid
> further expensive upcalls to the slow path, but subsequent packets
> dropped by the datapath are not accounted anywhere.
>
> Finally, the datapath itself drops packets in certain error
> situations.
> Also, these drops are today not accounted for.This makes it difficult
> for OVS users to monitor packet drop in an OVS instance and to alert a
> management system in case of a unexpected increase of such drops.
> Also OVS trouble-shooters face difficulties in analysing packet drops.
>
> With this patch we implement following changes to address the issues
> mentioned above.
>
> 1. Identify and account all the silent packet drop scenarios
>
> 2. Display these drops in ovs-appctl coverage/show
>
> v11->v12 Addresses comments from Ben Pfaff
>
> Co-authored-by: Rohith Basavaraja
> Co-authored-by: Keshav Gupta
> Signed-off-by: Anju Thomas
> Signed-off-by: Rohith Basavaraja
> Signed-off-by: Keshav Gupta
> ---
> datapath/linux/compat/include/linux/openvswitch.h | 1 +
> lib/dpif-netdev.c | 46 -
> lib/dpif.c| 7 +
> lib/dpif.h| 4 +
> lib/odp-execute.c | 79
> lib/odp-util.c| 9 +
> ofproto/ofproto-dpif-ipfix.c | 1 +
> ofproto/ofproto-dpif-sflow.c | 1 +
> ofproto/ofproto-dpif-xlate.c | 40 -
> ofproto/ofproto-dpif-xlate.h | 3 +
> ofproto/ofproto-dpif.c| 8 +
> ofproto/ofproto-dpif.h| 8 +-
> tests/automake.mk | 3 +-
> tests/dpif-netdev.at | 8 +
> tests/drop-stats.at | 210
> ++
> tests/ofproto-dpif.at | 2 +-
> tests/tunnel-push-pop.at | 29 ++-
> tests/tunnel.at | 16 +-
> 18 files changed, 464 insertions(+), 11 deletions(-) create mode
> 100644 tests/drop-stats.at
>
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> b/datapath/linux/compat/include/linux/openvswitch.h
> index 65a003a..415c053 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -989,6 +989,7 @@ enum ovs_action_attr { #ifndef __KERNEL__
> OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
> OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
> + OVS_ACTION_ATTR_DROP,
> #endif
> __OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
> * from userspace. */
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index
> 6b99a3c..3878b8d 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number
> of meters. */
> enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter.
> */
> enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
>
> +COVERAGE_DEFINE(datapath_drop_meter);
> +COVERAGE_DEFINE(datapath_drop_upcall_error);
> +COVERAGE_DEFINE(datapath_drop_lock_error);
> +COVERAGE_DEFINE(datapath_drop_userspace_action_error);
> +COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
> +COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
> +COVERAGE_DEFINE(datapath_drop_recirc_error);
> +COVERAGE_DEFINE(datapath_d
[ovs-dev] [PATCH v12] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on
port level. Packets that are dropped as part of normal OpenFlow
processing are counted in flow stats of “drop” flows or as table
misses in table stats. These can only be interpreted by controllers
that know the semantics of the configured OpenFlow pipeline.
Without that knowledge, it is impossible for an OVS user to obtain
e.g. the total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can
be dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid
further expensive upcalls to the slow path, but subsequent packets
dropped by the datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.This makes it difficult
for OVS users to monitor packet drop in an OVS instance and to alert
a management system in case of a unexpected increase of such drops.
Also OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
v11->v12 Addresses comments from Ben Pfaff
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 46 -
lib/dpif.c| 7 +
lib/dpif.h| 4 +
lib/odp-execute.c | 79
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 40 -
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 210 ++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 16 +-
18 files changed, 464 insertions(+), 11 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..415c053 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -989,6 +989,7 @@ enum ovs_action_attr {
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
OVS_ACTION_ATTR_TUNNEL_POP,/* u32 port number. */
+ OVS_ACTION_ATTR_DROP,
#endif
__OVS_ACTION_ATTR_MAX,/* Nothing past this will be accepted
* from userspace. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6b99a3c..3878b8d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5762,7 +5773,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band = &meter->bands[exceeded_band[j]];
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
-
+COVERAGE_INC(datapath_drop_meter);
dp_packet_delete(packet);
} else {
/* Meter accepts packet. */
@@ -6512,6 +6523,7 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd,
if (OVS_UNLIKELY(dp_packet_size(packet) < ETH_HEADER_LEN)) {
dp_packet_delete(packet);
+COVERAGE_INC(datapath_drop_rx_invalid_p
[ovs-dev] [PATCH v11] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level.
Packets that are dropped as part of normal OpenFlow processing are counted in
flow stats of “drop” flows or as table misses in table stats. These can only be
interpreted by controllers that know the semantics of the configured OpenFlow
pipeline. Without that knowledge, it is impossible for an OVS user to obtain
e.g. the total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be dropped
by OVS slow path that are not related to the OpenFlow pipeline.The generated
datapath flow entries include a drop action to avoid further expensive upcalls
to the slow path, but subsequent packets dropped by the datapath are not
accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.Also,
these drops are today not accounted for.This makes it difficult for OVS users
to monitor packet drop in an OVS instance and to alert a management system in
case of a unexpected increase of such drops. Also OVS trouble-shooters face
difficulties in analysing packet drops.
With this patch we implement following changes to address the issues mentioned
above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and link
for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
v11: addressed comments from Eelco and Gregory from v10
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 46 -
lib/dpif.c| 7 +
lib/dpif.h| 4 +
lib/odp-execute.c | 79
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 31 +++-
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 210 ++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 14 +-
18 files changed, 454 insertions(+), 10 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..d85e4dc 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -985,6 +985,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
+ OVS_ACTION_ATTR_DROP,
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4e73f96..b1b1fe0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -102,6 +102,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5650,7 +5661,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band = &meter->bands[exceeded_band[j]];
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
-
+COVERAGE_INC(datapath_drop_meter);
dp_packet_delete(packet);
} else {
/* Meter accepts packet
Re: [ovs-dev] [PATCH v10] Improved Packet Drop Statistics in OVS
Please disregard this path .. Sending an updated v10 -Original Message- From: 0-day Robot Sent: Friday, June 7, 2019 9:31 AM To: Anju Thomas Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v10] Improved Packet Drop Statistics in OVS Bleep bloop. Greetings Anju Thomas, 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. build: -e 's,[@]DBDIR[@],/usr/local/etc/openvswitch,g' \ -e 's,[@]PYTHON[@],/bin/python2,g' \ -e 's,[@]RUNDIR[@],/usr/local/var/run/openvswitch,g' \ -e 's,[@]VERSION[@],2.11.90,g' \ -e 's,[@]localstatedir[@],/usr/local/var,g' \ -e 's,[@]pkgdatadir[@],/usr/local/share/openvswitch,g' \ -e 's,[@]sysconfdir[@],/usr/local/etc,g' \ -e 's,[@]bindir[@],/usr/local/bin,g' \ -e 's,[@]sbindir[@],/usr/local/sbin,g' \ -e 's,[@]abs_builddir[@],/var/lib/jenkins/jobs/upstream_build_from_pw/workspace,g' \ -e 's,[@]abs_top_srcdir[@],/var/lib/jenkins/jobs/upstream_build_from_pw/workspace,g' \ > rhel/ovn-fedora.spec.tmp mv rhel/ovn-fedora.spec.tmp rhel/ovn-fedora.spec (printf '\043 Generated automatically -- do not modify!-*- buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.11.90,g') \ < ./xenserver/openvswitch-xen.spec.in > openvswitch-xen.spec.tmp || exit 1; \ if cmp -s openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; then touch xenserver/openvswitch-xen.spec; rm openvswitch-xen.spec.tmp; else mv openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; fi make[3]: Entering directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/datapath' make[3]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/datapath' The following files are in git but not the distribution: tests/drop-stats.at make[2]: *** [dist-hook-git] Error 1 make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace' make: *** [all] Error 2 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v10] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level.
Packets that are dropped as part of normal OpenFlow processing are counted in
flow stats of “drop” flows or as table misses in table stats.
These can only be interpreted by controllers that know the semantics of the
configured OpenFlow pipeline. Without that knowledge, it is impossible for an
OVS user to obtain e.g. the total number of packets dropped due to OpenFlow
rules.
Furthermore, there are numerous other reasons for which packets can be dropped
by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.
This makes it difficult for OVS users to monitor packet drop in an OVS instance
and to alert a management system in case of a unexpected increase of such
drops. Also OVS trouble-shooters face difficulties in analysing packet drops.
With this patch we implement following changes to address the issues mentioned
above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and link
for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 46 -
lib/dpif.c| 7 +
lib/dpif.h| 4 +
lib/odp-execute.c | 78
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 34 +++-
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 7 +
ofproto/ofproto-dpif.h| 8 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 211 ++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 14 +-
18 files changed, 455 insertions(+), 11 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..2de702e 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -985,6 +985,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
+OVS_ACTION_ATTR_DROP,
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5a6f2ab..ec72403 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5646,7 +5657,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band = &meter->bands[exceeded_band[j]];
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
-
+COVERAGE_INC(datapath_drop_meter);
dp_packet_delete(packet);
} else {
/* Meter accepts packet. */
@@ -6402,6 +6413,7 @@ dfc_processing(struct dp
[ovs-dev] [PATCH v10] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level.
Packets that are dropped as part of normal OpenFlow processing are counted in
flow stats of “drop” flows or as table misses in table stats. These can only be
interpreted by controllers that know the semantics of the configured OpenFlow
pipeline. Without that knowledge, it is impossible for an OVS user to obtain
e.g. the total number of packets dropped due to OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be dropped
by OVS slow path that are not related to the OpenFlow pipeline.The generated
datapath flow entries include a drop action to avoid further expensive upcalls
to the slow path, but subsequent packets dropped by the datapath are not
accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.Also,
these drops are today not accounted for.This makes it difficult for OVS users
to monitor packet drop in an OVS instance and to alert a management system in
case of a unexpected increase of such drops. Also OVS trouble-shooters face
difficulties in analysing packet drops.
With this patch we implement following changes to address the issues mentioned
above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and link
for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 46 -
lib/dpif.c| 7 +
lib/dpif.h| 4 +
lib/odp-execute.c | 78
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 34 +++-
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 7 +
ofproto/ofproto-dpif.h| 8 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 211 ++
tests/ofproto-dpif.at | 2 +-
tests/tunnel-push-pop.at | 29 ++-
tests/tunnel.at | 14 +-
17 files changed, 453 insertions(+), 10 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 65a003a..2de702e 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -985,6 +985,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
+OVS_ACTION_ATTR_DROP,
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5a6f2ab..ec72403 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5646,7 +5657,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band = &meter->bands[exceeded_band[j]];
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
-
+COVERAGE_INC(datapath_drop_meter);
dp_packet_delete(packet);
} else {
/* Meter accepts packet. */
@@ -6402,6 +6413,7 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd,
if (OVS_UNLIKELY(dp_packet_size(p
Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in OVS
Hi, Oh.. You are right I did send the wrong version. Let me send the latest version. Regards Anju -Original Message- From: Ilya Maximets Sent: Thursday, June 6, 2019 8:18 PM To: Anju Thomas ; ovs-dev@openvswitch.org Cc: Eelco Chaudron Subject: Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in OVS On 06.06.2019 17:28, Anju Thomas wrote: > Hi Ilya, > Yes, I have addressed those comments in v9. That is really strange. Maybe you've sent the wrong version? That is what I (and everyone else) received: https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356743.html It's the same code as down below. You may simply grep for following lines: COVERAGE_DEFINE(dp_sample_error_drop); <-- I asked to rename this. if (tnl_process_ecn(flow)<-- Here started a big diff that that I suggested to replace with a smaller one. packet_count = packets_->count; <-- Here I suggested to use dp_packet_batch_size(). Please, re-check. Here is my review for v8, just in case: https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356153.html Best regards, Ilya Maximets. > > Regards & Thanks > Anju > > -Original Message- > From: Ilya Maximets > Sent: Tuesday, June 4, 2019 12:33 PM > To: ovs-dev@openvswitch.org; Anju Thomas > Cc: Eelco Chaudron > Subject: Re: Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics > in OVS > >> Hi Eelco, >> Apart from Ilya's comments I have not received any comments for v8. I have >> addressed those comment in v9. > > Hi Anju. > I wrote about my comments to v8 just because you didn't reply or address a > bunch of them in v9. There was comments about using dp_packet_batch_size(), > renaming suggestion for sample and nsh drop counters, suggested ecn handling > code snippet, maybe something else. > > Best regards, Ilya Maximets. > >> >> Regards >> Anju >> >> -Original Message- >> From: Eelco Chaudron >> Sent: Monday, June 3, 2019 6:23 PM >> To: Anju Thomas >> Cc: dev at openvswitch.org >> Subject: Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in >> OVS >> >> Hi Anju, >> >> Wondering if you got the email below… >> >> Cheers, >> >> Eelco >> >> >> On 24 May 2019, at 11:46, Eelco Chaudron wrote: >> >>> Hi Anju, >>> >>> Was there ever a follow up on this patch? I only see one response >>> from Ilya on this asking about his v8 comments. >>> >>> Thanks, >>> >>> Eelco >>> >>> >>> On 27 Feb 2019, at 10:22, Anju Thomas wrote: >>> >>>> Currently OVS maintains explicit packet drop/error counters only on >>>> port level. Packets that are dropped as part of normal OpenFlow >>>> processing are counted in flow stats of “drop” flows or as table >>>> misses in table stats. >>>> These can only be interpreted by controllers that know the >>>> semantics of the configured OpenFlow pipeline. Without that >>>> knowledge, it is impossible for an OVS user to obtain e.g. the >>>> total number of packets dropped due to OpenFlow rules. >>>> >>>> Furthermore, there are numerous other reasons for which packets can >>>> be dropped by OVS slow path that are not related to the OpenFlow >>>> pipeline. >>>> The generated datapath flow entries include a drop action to avoid >>>> further expensive upcalls to the slow path, but subsequent packets >>>> dropped by the datapath are not accounted anywhere. >>>> >>>> Finally, the datapath itself drops packets in certain error >>>> situations. >>>> Also, these drops are today not accounted for. >>>> >>>> This makes it difficult for OVS users to monitor packet drop in an >>>> OVS instance and to alert a management system in case of a >>>> unexpected increase of such drops. Also OVS trouble-shooters face >>>> difficulties in analysing packet drops. >>>> >>>> With this patch we implement following changes to address the >>>> issues mentioned above. >>>> >>>> 1. Identify and account all the silent packet drop scenarios >>>> >>>> 2. Display these drops in ovs-appctl coverage/show >>>> >>>> A detailed presentation on this was presented at OvS conference >>>> 2017 and link for
Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in OVS
Hi Ilya, Yes, I have addressed those comments in v9. Regards & Thanks Anju -Original Message- From: Ilya Maximets Sent: Tuesday, June 4, 2019 12:33 PM To: ovs-dev@openvswitch.org; Anju Thomas Cc: Eelco Chaudron Subject: Re: Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in OVS > Hi Eelco, > Apart from Ilya's comments I have not received any comments for v8. I have > addressed those comment in v9. Hi Anju. I wrote about my comments to v8 just because you didn't reply or address a bunch of them in v9. There was comments about using dp_packet_batch_size(), renaming suggestion for sample and nsh drop counters, suggested ecn handling code snippet, maybe something else. Best regards, Ilya Maximets. > > Regards > Anju > > -Original Message- > From: Eelco Chaudron > Sent: Monday, June 3, 2019 6:23 PM > To: Anju Thomas > Cc: dev at openvswitch.org > Subject: Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in > OVS > > Hi Anju, > > Wondering if you got the email below… > > Cheers, > > Eelco > > > On 24 May 2019, at 11:46, Eelco Chaudron wrote: > >> Hi Anju, >> >> Was there ever a follow up on this patch? I only see one response >> from Ilya on this asking about his v8 comments. >> >> Thanks, >> >> Eelco >> >> >> On 27 Feb 2019, at 10:22, Anju Thomas wrote: >> >>> Currently OVS maintains explicit packet drop/error counters only on >>> port level. Packets that are dropped as part of normal OpenFlow >>> processing are counted in flow stats of “drop” flows or as table >>> misses in table stats. >>> These can only be interpreted by controllers that know the semantics >>> of the configured OpenFlow pipeline. Without that knowledge, it is >>> impossible for an OVS user to obtain e.g. the total number of >>> packets dropped due to OpenFlow rules. >>> >>> Furthermore, there are numerous other reasons for which packets can >>> be dropped by OVS slow path that are not related to the OpenFlow >>> pipeline. >>> The generated datapath flow entries include a drop action to avoid >>> further expensive upcalls to the slow path, but subsequent packets >>> dropped by the datapath are not accounted anywhere. >>> >>> Finally, the datapath itself drops packets in certain error >>> situations. >>> Also, these drops are today not accounted for. >>> >>> This makes it difficult for OVS users to monitor packet drop in an >>> OVS instance and to alert a management system in case of a >>> unexpected increase of such drops. Also OVS trouble-shooters face >>> difficulties in analysing packet drops. >>> >>> With this patch we implement following changes to address the issues >>> mentioned above. >>> >>> 1. Identify and account all the silent packet drop scenarios >>> >>> 2. Display these drops in ovs-appctl coverage/show >>> >>> A detailed presentation on this was presented at OvS conference 2017 >>> and link for the corresponding presentation is available at: >>> >>> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the >>> - >>> data-plane-in-ovs-82280329 >>> >>> Co-authored-by: Rohith Basavaraja >>> Co-authored-by: Keshav Gupta >>> Signed-off-by: Anju Thomas >>> Signed-off-by: Rohith Basavaraja >>> Signed-off-by: Keshav Gupta >>> --- >>> datapath/linux/compat/include/linux/openvswitch.h | 1 + >>> lib/dpif-netdev.c | 44 - >>> lib/dpif.c| 7 + >>> lib/dpif.h| 3 + >>> lib/odp-execute.c | 81 - >>> lib/odp-util.c| 9 + >>> ofproto/ofproto-dpif-ipfix.c | 1 + >>> ofproto/ofproto-dpif-sflow.c | 1 + >>> ofproto/ofproto-dpif-xlate.c | 103 +++ >>> ofproto/ofproto-dpif-xlate.h | 3 + >>> ofproto/ofproto-dpif.c| 8 + >>> ofproto/ofproto-dpif.h| 7 +- >>> tests/automake.mk | 3 +- >>> tests/dpif-netdev.at | 8 + >>> tests/drop-stats.at | 197 >>> ++ >>> tests/ofpro
Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in OVS
Hi Eelco,
Apart from Ilya's comments I have not received any comments for v8. I have
addressed those comment in v9.
Regards
Anju
-Original Message-
From: Eelco Chaudron
Sent: Monday, June 3, 2019 6:23 PM
To: Anju Thomas
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v9] Improved Packet Drop Statistics in OVS
Hi Anju,
Wondering if you got the email below…
Cheers,
Eelco
On 24 May 2019, at 11:46, Eelco Chaudron wrote:
> Hi Anju,
>
> Was there ever a follow up on this patch? I only see one response from
> Ilya on this asking about his v8 comments.
>
> Thanks,
>
> Eelco
>
>
> On 27 Feb 2019, at 10:22, Anju Thomas wrote:
>
>> Currently OVS maintains explicit packet drop/error counters only on
>> port level. Packets that are dropped as part of normal OpenFlow
>> processing are counted in flow stats of “drop” flows or as table
>> misses in table stats.
>> These can only be interpreted by controllers that know the semantics
>> of the configured OpenFlow pipeline. Without that knowledge, it is
>> impossible for an OVS user to obtain e.g. the total number of packets
>> dropped due to OpenFlow rules.
>>
>> Furthermore, there are numerous other reasons for which packets can
>> be dropped by OVS slow path that are not related to the OpenFlow
>> pipeline.
>> The generated datapath flow entries include a drop action to avoid
>> further expensive upcalls to the slow path, but subsequent packets
>> dropped by the datapath are not accounted anywhere.
>>
>> Finally, the datapath itself drops packets in certain error
>> situations.
>> Also, these drops are today not accounted for.
>>
>> This makes it difficult for OVS users to monitor packet drop in an
>> OVS instance and to alert a management system in case of a unexpected
>> increase of such drops. Also OVS trouble-shooters face difficulties
>> in analysing packet drops.
>>
>> With this patch we implement following changes to address the issues
>> mentioned above.
>>
>> 1. Identify and account all the silent packet drop scenarios
>>
>> 2. Display these drops in ovs-appctl coverage/show
>>
>> A detailed presentation on this was presented at OvS conference 2017
>> and link for the corresponding presentation is available at:
>>
>> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-
>> data-plane-in-ovs-82280329
>>
>> Co-authored-by: Rohith Basavaraja
>> Co-authored-by: Keshav Gupta
>> Signed-off-by: Anju Thomas
>> Signed-off-by: Rohith Basavaraja
>> Signed-off-by: Keshav Gupta
>> ---
>> datapath/linux/compat/include/linux/openvswitch.h | 1 +
>> lib/dpif-netdev.c | 44 -
>> lib/dpif.c| 7 +
>> lib/dpif.h| 3 +
>> lib/odp-execute.c | 81 -
>> lib/odp-util.c| 9 +
>> ofproto/ofproto-dpif-ipfix.c | 1 +
>> ofproto/ofproto-dpif-sflow.c | 1 +
>> ofproto/ofproto-dpif-xlate.c | 103 +++
>> ofproto/ofproto-dpif-xlate.h | 3 +
>> ofproto/ofproto-dpif.c| 8 +
>> ofproto/ofproto-dpif.h| 7 +-
>> tests/automake.mk | 3 +-
>> tests/dpif-netdev.at | 8 +
>> tests/drop-stats.at | 197
>> ++
>> tests/ofproto-dpif.at | 2 +-
>> tests/testsuite.at| 1 +
>> tests/tunnel-push-pop.at | 28 ++-
>> tests/tunnel.at | 14 +-
>> 19 files changed, 475 insertions(+), 46 deletions(-) create mode
>> 100644 tests/drop-stats.at
>>
>> diff --git a/datapath/linux/compat/include/linux/openvswitch.h
>> b/datapath/linux/compat/include/linux/openvswitch.h
>> index d5aa09d..e77e9c8 100644
>> --- a/datapath/linux/compat/include/linux/openvswitch.h
>> +++ b/datapath/linux/compat/include/linux/openvswitch.h
>> @@ -946,6 +946,7 @@ enum ovs_action_attr {
>> OVS_ACTION_ATTR_POP_NSH, /* No argument. */
>> OVS_ACTION_ATTR_METER,/* u32 meter number. */
>> OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
>> +OVS_ACTION_ATTR_DROP, /* Drop action. */
>>
>> #ifndef __KERNEL__
>> OVS_ACTION_ATT
[ovs-dev] [PATCH v1] Incorrect match criteria for in-band control rule
As part of in-band control, OVS is expected to send DHCP server replies to the
LOCAL port as well. In this case, OVS implicitly adds an additional action to
output to the bridge’s LOCAL port after the ofproto translation for the packet
is completed in the ofproto layer but before sending the actions to datapath
for installation.
However, the match criteria is unchanged and as a result all packets (not just
DHCP server replies) are also sent to the LOCAL port.
The fix is to add the IP protocol type (UDP), the UDP source and destination
ports to the match criteria so that a specific datapath flow that matches only
DHCP server replies is installed. As a result, only DHCP server reply packets
will be sent to the LOCAL port.
Signed-off-by: Anju Thomas
---
ofproto/ofproto-dpif-xlate.c | 4
1 file changed, 4 insertions(+)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ae8b999..04d69ed 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -7584,6 +7584,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
&& xbridge->has_in_band
&& in_band_must_output_to_local_port(flow)
&& !actions_output_to_local_port(&ctx)) {
+WC_MASK_FIELD(ctx.wc, nw_proto);
+WC_MASK_FIELD(ctx.wc, tp_src);
+WC_MASK_FIELD(ctx.wc, tp_dst);
+WC_MASK_FIELD(ctx.wc, dl_type);
compose_output_action(&ctx, OFPP_LOCAL, NULL, false, false);
}
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] Fix crash due to multiple tnl push action
So then should we refrain from adding this fix to 2.9 if it does not make sense
or should I remove the cases altogether?
Regards
Anju
-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Wednesday, February 27, 2019 6:55 PM
To: Anju Thomas ; d...@openvswitch.org
Cc: Ben Pfaff
Subject: Re: [ovs-dev] Fix crash due to multiple tnl push action
Hi.
Thanks for the patch.
Few comments:
1. You may use --subject-prefix="PATCH branch-2.9". In this case ovsrobot
will be able to apply patch to the right branch for checking.
2. This patch fixes tests (I didn't check), but it effectively makes them
useless by replacing all the useful data by simple 'drop' action.
It's the same as just removing the broken test.
Best regards, Ilya Maximets.
On 27.02.2019 14:41, Anju Thomas wrote:
> During slow path packet processing, if the action is to output to a
> tunnel port, the slow path processing of the encapsulated packet
> continues on the underlay bridge and additional actions (e.g. optional
> VLAN encapsulation, bond link selection and finally output to port)
> are collected there.
>
> To prepare for a continuation of the processing of the original packet
> (e.g. output to other tunnel ports in a flooding scenario), the
> “tunnel_push” action and the actions of the underlay bridge are
> encapsulated in a clone() action to preserve the original packet.
>
> If the underlay bridge decides to drop the tunnel packet (for example
> if both bonded ports are down simultaneously), the clone(tunnel_push))
> actions previously generated as part of translation of the output to
> tunnel port are discarded and a stand-alone tunnel_push action is
> added instead. Thus the tunnel header is pushed on to the original packet.
> This is the bug.
>
> Consequences: If packet processing continues with sending to further
> tunnel ports, multiple tunnel header pushes will happen on the
> original packet as typically the tunnels all traverse the same
> underlay bond which is down. The packet may not have enough headroom
> to accommodate all the tunnel headers. OVS crashes if it runs out of
> space while trying to push the tunnel headers.
>
> Even in case there is enough headroom, the packet will not be freed
> since the accumulated action list contains only the tunnel header push
> action without any output port action. Thus, we either have a crash or
> a packet buffer leak.
>
> Signed-off-by: Anju Thomas
> ---
> ofproto/ofproto-dpif-xlate.c | 7 ---
> tests/tunnel-push-pop-ipv6.at | 10 +-
> tests/tunnel-push-pop.at | 18 +-
> 3 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c
> b/ofproto/ofproto-dpif-xlate.c index ed8..7e69469 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3510,13 +3510,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const
> struct xport *xport,
> nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
> } else {
> nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
> -/* XXX : There is no real use-case for a tunnel push without
> - * any post actions. However keeping it now
> - * as is to make the 'make check' happy. Should remove when all
> the
> - * make check tunnel test case does something meaningful on a
> - * tunnel encap packets.
> - */
> -odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
> }
>
> /* Restore context status. */ diff --git
> a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at index
> 7ca522a..c9e4a2b 100644
> --- a/tests/tunnel-push-pop-ipv6.at
> +++ b/tests/tunnel-push-pop-ipv6.at
> @@ -93,28 +93,28 @@ dnl Check VXLAN tunnel push AT_CHECK([ovs-ofctl
> add-flow int-br action=2]) AT_CHECK([ovs-appctl ofproto/trace
> ovs-dummy
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(
> 0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)
> '], [0], [stdout]) AT_CHECK([tail -1 stdout], [0],
> - [Datapath actions:
> tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b
> 6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=200
> 1:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,c
> sum=0x),vxlan(flags=0x800,vni=0x7b)),out_port(100))
> + [Datapath actions: drop
> ])
>
> dnl Check VXLAN tunnel push set tunnel id by flow and checksum
> AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])
> AT_CHECK([ovs-appctl ofproto/trace ovs-dummy
[ovs-dev] [PATCH] Fix crash due to multiple tnl push action
During slow path packet processing, if the action is to output to a
tunnel port, the slow path processing of the encapsulated packet
continues on the underlay bridge and additional actions (e.g. optional
VLAN encapsulation, bond link selection and finally output to port) are
collected there.
To prepare for a continuation of the processing of the original packet
(e.g. output to other tunnel ports in a flooding scenario), the
“tunnel_push” action and the actions of the underlay bridge are
encapsulated in a clone() action to preserve the original packet.
If the underlay bridge decides to drop the tunnel packet (for example if
both bonded ports are down simultaneously), the clone(tunnel_push))
actions previously generated as part of translation of the output to
tunnel port are discarded and a stand-alone tunnel_push action is added
instead. Thus the tunnel header is pushed on to the original packet.
This is the bug.
Consequences: If packet processing continues with sending to further
tunnel ports, multiple tunnel header pushes will happen on the original
packet as typically the tunnels all traverse the same underlay bond
which is down. The packet may not have enough headroom to accommodate
all the tunnel headers. OVS crashes if it runs out of space while trying
to push the tunnel headers.
Even in case there is enough headroom, the packet will not be freed
since the accumulated action list contains only the tunnel header push
action without any output port action. Thus, we either have a crash or a
packet buffer leak.
Signed-off-by: Anju Thomas
---
ofproto/ofproto-dpif-xlate.c | 7 ---
tests/tunnel-push-pop-ipv6.at | 10 +-
tests/tunnel-push-pop.at | 18 +-
3 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ed8..7e69469 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3510,13 +3510,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct
xport *xport,
nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
} else {
nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
-/* XXX : There is no real use-case for a tunnel push without
- * any post actions. However keeping it now
- * as is to make the 'make check' happy. Should remove when all the
- * make check tunnel test case does something meaningful on a
- * tunnel encap packets.
- */
-odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
}
/* Restore context status. */
diff --git a/tests/tunnel-push-pop-ipv6.at b/tests/tunnel-push-pop-ipv6.at
index 7ca522a..c9e4a2b 100644
--- a/tests/tunnel-push-pop-ipv6.at
+++ b/tests/tunnel-push-pop-ipv6.at
@@ -93,28 +93,28 @@ dnl Check VXLAN tunnel push
AT_CHECK([ovs-ofctl add-flow int-br action=2])
AT_CHECK([ovs-appctl ofproto/trace ovs-dummy
'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
[0], [stdout])
AT_CHECK([tail -1 stdout], [0],
- [Datapath actions:
tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7b)),out_port(100))
+ [Datapath actions: drop
])
dnl Check VXLAN tunnel push set tunnel id by flow and checksum
AT_CHECK([ovs-ofctl add-flow int-br "actions=set_tunnel:124,4"])
AT_CHECK([ovs-appctl ofproto/trace ovs-dummy
'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
[0], [stdout])
AT_CHECK([tail -1 stdout], [0],
- [Datapath actions:
tnl_push(tnl_port(4789),header(size=70,type=4,eth(dst=f8:bc:12:44:34:b7,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::93,label=0,proto=17,tclass=0x0,hlimit=64),udp(src=0,dst=4789,csum=0x),vxlan(flags=0x800,vni=0x7c)),out_port(100))
+ [Datapath actions: drop
])
dnl Check GRE tunnel push
AT_CHECK([ovs-ofctl add-flow int-br action=3])
AT_CHECK([ovs-appctl ofproto/trace ovs-dummy
'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
[0], [stdout])
AT_CHECK([tail -1 stdout], [0],
- [Datapath actions:
tnl_push(tnl_port(3),header(size=62,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=47,tclass=0x0,hlimit=64),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100))
+ [Datapath actions: drop
])
dnl Check Geneve tunnel push
AT_CHECK([ovs-ofctl add-flow int-br
"actions=set_field:2001:cafe::92->tun_ipv6_
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Sure Ben. I will send a patch for 2.9 branch.
Regards
Anju
-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org]
Sent: Tuesday, February 26, 2019 8:55 PM
To: Ilya Maximets
Cc: ovs-dev@openvswitch.org; Anju Thomas ; Stokes,
Ian
Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Thanks for the report.
For now, I reverted this from branch-2.9. I'd be pleased to have a fixed patch
for branch-2.9, though.
On Tue, Feb 26, 2019 at 03:22:08PM +0300, Ilya Maximets wrote:
> Hi Ben, Anju.
>
> This patch breaks tunnel_push_pop unit tests on branch-2.9:
>
> tunnel_push_pop
>
> 791: tunnel_push_pop - actionFAILED
> (tunnel-push-pop.at:115)
> 792: tunnel_push_pop - packet_outok
> 793: tunnel_push_pop - underlay bridge match FAILED
> (tunnel-push-pop.at:327)
>
>
> ./tunnel-push-pop.at:114: ovs-appctl ofproto/trace ovs-dummy
> 'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'
> stdout:
> Flow:
> ip,in_port=LOCAL,vlan_tci=0x,dl_src=f8:bc:12:44:34:b6,dl_dst=aa:55
> :aa:55:00:00,nw_src=1.1.3.88,nw_dst=1.1.3.112,nw_proto=47,nw_tos=0,nw_
> ecn=0,nw_ttl=64
>
> bridge("int-br")
>
> 0. priority 32768
> output:2
> -> output to native tunnel
> -> tunneling to 1.1.2.92 via br0
> -> tunneling from aa:55:aa:55:00:00 1.1.2.88 to f8:bc:12:44:34:b6
> 1.1.2.92
>
> bridge("br0")
> -
> 0. priority 32768
> NORMAL
> -> learned port is input port, dropping
>
> Final flow: unchanged
> Megaflow: recirc_id=0,eth,ip,in_port=LOCAL,nw_ecn=0,nw_frag=no
> Datapath actions: drop
> ./tunnel-push-pop.at:115: tail -1 stdout
> --- - 2019-02-26 15:14:47.663428963 +0300
> +++ /tests/testsuite.dir/at-groups/791/stdout2019-02-26
> 15:14:47.659754269 +0300
> @@ -1,2 +1,2 @@
> -Datapath actions:
> tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:b
> 6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92
> ,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=4789,csum=0x0),vxlan
> (flags=0x800,vni=0x7b)),out_port(100))
> +Datapath actions: drop
>
>
> Could you please take a look?
>
> Best regards, Ilya Maximets.
>
> > OK. I applied the first part to all relevant branches.
> >
> > On Mon, Jan 21, 2019 at 04:06:26AM +, Anju Thomas wrote:
> >> Hi Ben/Ian,
> >> I understand that the second part of the patch needs more review.
> >> But do you think we can atleast merge the first part of this viz
> >> https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346649.html so
> >> that to crash and more importantly silent leaks . We can widely review the
> >> other part of the code as it might require more indepth testing and review.
> >>
> >> What are your suggestions ?
> >>
> >> Regards
> >> Anju
> >>
> >> -Original Message-
> >> From: Ben Pfaff [mailto:blp at ovn.org]
> >> Sent: Thursday, January 17, 2019 11:39 PM
> >> To: Stokes, Ian
> >> Cc: Lam, Tiago ; Anju Thomas >> at ericsson.com>; dev at openvswitch.org
> >> Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl
> >> push action
> >>
> >> On Wed, Jan 16, 2019 at 11:38:38AM +, Stokes, Ian wrote:
> >> > > On 16/01/2019 09:30, Anju Thomas wrote:
> >> > > >
> >> > > > Hi Folks,
> >> > > >
> >> > > > Are these changes planned to be merged as well?
> >> > > >
> >> > > > Regards
> >> > > > Anju
> >> > >
> >> > > Hi Anju,
> >> > >
> >> > > Unfortunately, no. An RFC based on the below was proposed to
> >> > > the mailing list here [1], but no discussion / comments
> >> > > happened after that. Further discussion and testing would be needed to
> >> > > move this forward...
> >> > >
> >> >
> >> > I wasn't aware of this tbh. It seems like a bug fix so I believe it
> >> > would be eligible to be merged for the 2.11 release a(and possibly
> >> > backported also).
> >> >
> >> > As it manipulates the dp packets functionality it would need wider
> >> > testing and discussion among the community.
> >> >
> >> > It won't make it upstream this week before we cut for 2.11 I would think
> >> > but there is 4 weeks allowed until release where bug fixes are allowed.
> >> > If further work continues it could be merged within that window or
> >> > afterwards and backported?
> >>
> >> Bug fixes are welcomed at any time, pre- or post-release.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v9] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing are
counted in flow stats of “drop” flows or as table misses in table stats.
These can only be interpreted by controllers that know the semantics of
the configured OpenFlow pipeline. Without that knowledge, it is impossible
for an OVS user to obtain e.g. the total number of packets dropped due to
OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.
This makes it difficult for OVS users to monitor packet drop in an OVS
instance and to alert a management system in case of a unexpected increase
of such drops. Also OVS trouble-shooters face difficulties in analysing
packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and
link for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 44 -
lib/dpif.c| 7 +
lib/dpif.h| 3 +
lib/odp-execute.c | 81 -
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 103 +++
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 7 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 197 ++
tests/ofproto-dpif.at | 2 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 ++-
tests/tunnel.at | 14 +-
19 files changed, 475 insertions(+), 46 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index d5aa09d..e77e9c8 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -946,6 +946,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_POP_NSH, /* No argument. */
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
+ OVS_ACTION_ATTR_DROP, /* Drop action. */
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 77ac1d2..acc7913 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5647,6 +5658,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
+COVERAGE_INC(datapath_drop_meter);
dp_packet_delete(packet);
} else {
/* Meter accepts packet. */
@@ -6402,6 +6414,7 @@ dfc_processing(struct dp_netdev
[ovs-dev] [PATCHv8] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing are
counted in flow stats of “drop” flows or as table misses in table stats.
These can only be interpreted by controllers that know the semantics of
the configured OpenFlow pipeline. Without that knowledge, it is impossible
for an OVS user to obtain e.g. the total number of packets dropped due to
OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.
This makes it difficult for OVS users to monitor packet drop in an OVS
instance and to alert a management system in case of a unexpected increase
of such drops. Also OVS trouble-shooters face difficulties in analysing
packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and
link for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 44 -
lib/dpif.c| 7 +
lib/dpif.h| 3 +
lib/odp-execute.c | 81 -
lib/odp-util.c| 9 +
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 101 +++
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 7 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 197 ++
tests/ofproto-dpif.at | 2 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 ++-
tests/tunnel.at | 14 +-
19 files changed, 473 insertions(+), 46 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index d5aa09d..e77e9c8 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -946,6 +946,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_POP_NSH, /* No argument. */
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
+ OVS_ACTION_ATTR_DROP, /* Drop action. */
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 77ac1d2..acc7913 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -101,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5647,6 +5658,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
+COVERAGE_INC(datapath_drop_meter);
dp_packet_delete(packet);
} else {
/* Meter accepts packet. */
@@ -6402,6 +6414,7 @@ dfc_processing(struct dp_netdev
Re: [ovs-dev] [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
Thanks Ilya.
I have sent the updated patch.
Regards
Anju
-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Thursday, February 07, 2019 7:52 PM
To: Anju Thomas ; Ben Pfaff
Cc: d...@openvswitch.org
Subject: Re: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
On 07.02.2019 17:11, Anju Thomas wrote:
> Thanks for comments Ilya. Just one question regarding the steal flag
> :-
>
>> +case OVS_ACTION_ATTR_DROP: {
>> +const enum xlate_error *drop_reason = nl_attr_get(a);
>> +if (*drop_reason < XLATE_MAX) {
>> + dp_update_drop_action_counter(*drop_reason, batch->count);
>> +}
>> +dp_packet_delete_batch(batch, steal);
>
> I'm not sure if we need to honor 'steal' while performing the DROP action,
> because packets will not be dropped if 'steal == false'.
>
> What if there is the drop is within a clone action and there is
> another output action.In that case , we would not want to delete the batch .
> Right?
If the drop action is within a clone action, it will drop cloned packets.
i.e. "actions=clone(drop),output:N" should clone packets, drop cloned packets
and send original packets to port N.
"actions=drop,output:N" should not send anything to N, because all packets
should be dropped while executing drop action. This list of actions makes no
practical sense, though.
>
> Regards
> Anju
>
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Wednesday, February 06, 2019 8:48 PM
> To: Anju Thomas ; Ben Pfaff
> Cc: d...@openvswitch.org
> Subject: Re: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
>
> Hi.
> See comments inline.
>
> Best regards, Ilya Maximets.
>
> On 06.02.2019 7:01, Anju Thomas wrote:
>>
>> Hi Ben/Ilya,
>>
>> I have addressed the comments in the below patch. Can you tell me if
>> this is fin> Regards Anju -Original Message-
>> From: Anju Thomas [mailto:anju.tho...@ericsson.com]
>> Sent: Tuesday, January 29, 2019 5:21 PM
>> To: d...@openvswitch.org
>> Cc: Anju Thomas
>> Subject: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
>>
>>Currently OVS maintains explicit packet drop/error counters only on port
>>level. Packets that are dropped as part of normal OpenFlow processing are
>>counted in flow stats of “drop” flows or as table misses in table stats.
>>These can only be interpreted by controllers that know the semantics of
>>the configured OpenFlow pipeline. Without that knowledge, it is impossible
>>for an OVS user to obtain e.g. the total number of packets dropped due to
>>OpenFlow rules.
>>
>>Furthermore, there are numerous other reasons for which packets can be
>>dropped by OVS slow path that are not related to the OpenFlow pipeline.
>>The generated datapath flow entries include a drop action to avoid further
>>expensive upcalls to the slow path, but subsequent packets dropped by the
>>datapath are not accounted anywhere.
>>
>>Finally, the datapath itself drops packets in certain error situations.
>>Also, these drops are today not accounted for.
>>
>>This makes it difficult for OVS users to monitor packet drop in an OVS
>>instance and to alert a management system in case of a unexpected increase
>>of such drops. Also OVS trouble-shooters face difficulties in analysing
>>packet drops.
>>
>>With this patch we implement following changes to address the issues
>>mentioned above.
>>
>>1. Identify and account all the silent packet drop scenarios
>>
>>2. Display these drops in ovs-appctl coverage/show
>>
>>A detailed presentation on this was presented at OvS conference 2017 and
>>link for the corresponding presentation is available at:
>>
>>
>> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-
>> d
>> ata-plane-in-ovs-82280329
>>
>>Co-authored-by: Rohith Basavaraja
>>Co-authored-by: Keshav Gupta
>>Signed-off-by: Anju Thomas
>>Signed-off-by: Rohith Basavaraja
>>Signed-off-by: Keshav Gupta
>
>
> You still have this strange shift to the right by 3 spaces in commit-message.
>
>> ---
>> datapath/linux/compat/include/linux/openvswitch.h | 1 +
>> lib/dpif-netdev.c | 39 ++-
>> lib/dpif.c| 7 ++
>> lib/dpif.h| 3 +
>> lib/netdev-
[ovs-dev] [PATCHv8 1/3] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing are
counted in flow stats of “drop” flows or as table misses in table stats.
These can only be interpreted by controllers that know the semantics of
the configured OpenFlow pipeline. Without that knowledge, it is impossible
for an OVS user to obtain e.g. the total number of packets dropped due to
OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.
This makes it difficult for OVS users to monitor packet drop in an OVS
instance and to alert a management system in case of a unexpected increase
of such drops. Also OVS trouble-shooters face difficulties in analysing
packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and
link for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 44 -
lib/dpif.c| 7 +
lib/dpif.h| 3 +
lib/odp-execute.c | 81 +-
lib/odp-util.c| 9 ++
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 101 +++-
ofproto/ofproto-dpif-xlate.h | 3 +
ofproto/ofproto-dpif.c| 8 +
ofproto/ofproto-dpif.h| 7 +-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 8 +
tests/drop-stats.at | 189 ++
tests/ofproto-dpif.at | 2 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 +++-
tests/tunnel.at | 14 +-
19 files changed, 465 insertions(+), 46 deletions(-)
create mode 100644 tests/drop-stats.at
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 9b087f1..559b296 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -938,6 +938,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_POP_NSH, /* No argument. */
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
+ OVS_ACTION_ATTR_DROP, /* Drop action. */
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0f57e3f..9d03de4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -100,6 +100,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5644,6 +5655,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band->packet_count += 1;
band->byte_count += dp_packet_size(packet);
+COVERAGE_INC(datapath_drop_meter);
dp_packet_delete(packet);
} else {
/* Meter accepts packet. */
@@ -6399,6 +6411,7 @@ dfc_processing(struct dp_n
Re: [ovs-dev] [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
Thanks for comments Ilya. Just one question regarding the steal flag :-
> +case OVS_ACTION_ATTR_DROP: {
> +const enum xlate_error *drop_reason = nl_attr_get(a);
> +if (*drop_reason < XLATE_MAX) {
> + dp_update_drop_action_counter(*drop_reason, batch->count);
> +}
> +dp_packet_delete_batch(batch, steal);
I'm not sure if we need to honor 'steal' while performing the DROP action,
because packets will not be dropped if 'steal == false'.
What if there is the drop is within a clone action and there is another
output action.In that case , we would not want to delete the batch . Right?
Regards
Anju
-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Wednesday, February 06, 2019 8:48 PM
To: Anju Thomas ; Ben Pfaff
Cc: d...@openvswitch.org
Subject: Re: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
Hi.
See comments inline.
Best regards, Ilya Maximets.
On 06.02.2019 7:01, Anju Thomas wrote:
>
> Hi Ben/Ilya,
>
> I have addressed the comments in the below patch. Can you tell me if
> this is fin> Regards Anju -Original Message-
> From: Anju Thomas [mailto:anju.tho...@ericsson.com]
> Sent: Tuesday, January 29, 2019 5:21 PM
> To: d...@openvswitch.org
> Cc: Anju Thomas
> Subject: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
>
>Currently OVS maintains explicit packet drop/error counters only on port
>level. Packets that are dropped as part of normal OpenFlow processing are
>counted in flow stats of “drop” flows or as table misses in table stats.
>These can only be interpreted by controllers that know the semantics of
>the configured OpenFlow pipeline. Without that knowledge, it is impossible
>for an OVS user to obtain e.g. the total number of packets dropped due to
>OpenFlow rules.
>
>Furthermore, there are numerous other reasons for which packets can be
>dropped by OVS slow path that are not related to the OpenFlow pipeline.
>The generated datapath flow entries include a drop action to avoid further
>expensive upcalls to the slow path, but subsequent packets dropped by the
>datapath are not accounted anywhere.
>
>Finally, the datapath itself drops packets in certain error situations.
>Also, these drops are today not accounted for.
>
>This makes it difficult for OVS users to monitor packet drop in an OVS
>instance and to alert a management system in case of a unexpected increase
>of such drops. Also OVS trouble-shooters face difficulties in analysing
>packet drops.
>
>With this patch we implement following changes to address the issues
>mentioned above.
>
>1. Identify and account all the silent packet drop scenarios
>
>2. Display these drops in ovs-appctl coverage/show
>
>A detailed presentation on this was presented at OvS conference 2017 and
>link for the corresponding presentation is available at:
>
>
> https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-d
> ata-plane-in-ovs-82280329
>
>Co-authored-by: Rohith Basavaraja
>Co-authored-by: Keshav Gupta
>Signed-off-by: Anju Thomas
>Signed-off-by: Rohith Basavaraja
>Signed-off-by: Keshav Gupta
You still have this strange shift to the right by 3 spaces in commit-message.
> ---
> datapath/linux/compat/include/linux/openvswitch.h | 1 +
> lib/dpif-netdev.c | 39 ++-
> lib/dpif.c| 7 ++
> lib/dpif.h| 3 +
> lib/netdev-dpdk.c | 4 ++
> lib/odp-execute.c | 81
> +++
> lib/odp-execute.h | 2 +
> lib/odp-util.c| 10 ++-
> ofproto/ofproto-dpif-ipfix.c | 1 +
> ofproto/ofproto-dpif-sflow.c | 1 +
> ofproto/ofproto-dpif-xlate.c | 31 +
> ofproto/ofproto-dpif-xlate.h | 4 ++
> ofproto/ofproto-dpif.c| 8 +++
> ofproto/ofproto-dpif.h| 8 ++-
> tests/automake.mk | 3 +-
> tests/dpif-netdev.at | 10 +++
> tests/ofproto-dpif.at | 4 +-
> tests/testsuite.at| 1 +
> tests/tunnel-push-pop.at | 28 +++-
> tests/tunnel.at | 14 +++-
> 20 files changed, 248 insertions(+), 12 deletions(-)
>
> diff --git a/datapath/lin
Re: [ovs-dev] [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
Hi Ben/Ilya,
I have addressed the comments in the below patch. Can you tell me if this is
fine?
Regards
Anju
-Original Message-
From: Anju Thomas [mailto:anju.tho...@ericsson.com]
Sent: Tuesday, January 29, 2019 5:21 PM
To: d...@openvswitch.org
Cc: Anju Thomas
Subject: [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing are
counted in flow stats of “drop” flows or as table misses in table stats.
These can only be interpreted by controllers that know the semantics of
the configured OpenFlow pipeline. Without that knowledge, it is impossible
for an OVS user to obtain e.g. the total number of packets dropped due to
OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.
This makes it difficult for OVS users to monitor packet drop in an OVS
instance and to alert a management system in case of a unexpected increase
of such drops. Also OVS trouble-shooters face difficulties in analysing
packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and
link for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 39 ++-
lib/dpif.c| 7 ++
lib/dpif.h| 3 +
lib/netdev-dpdk.c | 4 ++
lib/odp-execute.c | 81 +++
lib/odp-execute.h | 2 +
lib/odp-util.c| 10 ++-
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 31 +
ofproto/ofproto-dpif-xlate.h | 4 ++
ofproto/ofproto-dpif.c| 8 +++
ofproto/ofproto-dpif.h| 8 ++-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 10 +++
tests/ofproto-dpif.at | 4 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 +++-
tests/tunnel.at | 14 +++-
20 files changed, 248 insertions(+), 12 deletions(-)
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 9b087f1..92db378 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -938,6 +938,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_POP_NSH, /* No argument. */
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
+ OVS_ACTION_ATTR_DROP,
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 0f57e3f..c726463 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -77,6 +77,7 @@
#include "unixctl.h"
#include "util.h"
#include "uuid.h"
+#include "ofproto/ofproto-dpif-xlate.h"
VLOG_DEFINE_THIS_MODULE(dpif_netdev);
@@ -100,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERA
[ovs-dev] [PATCHv7 1/3] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port
level. Packets that are dropped as part of normal OpenFlow processing are
counted in flow stats of “drop” flows or as table misses in table stats.
These can only be interpreted by controllers that know the semantics of
the configured OpenFlow pipeline. Without that knowledge, it is impossible
for an OVS user to obtain e.g. the total number of packets dropped due to
OpenFlow rules.
Furthermore, there are numerous other reasons for which packets can be
dropped by OVS slow path that are not related to the OpenFlow pipeline.
The generated datapath flow entries include a drop action to avoid further
expensive upcalls to the slow path, but subsequent packets dropped by the
datapath are not accounted anywhere.
Finally, the datapath itself drops packets in certain error situations.
Also, these drops are today not accounted for.
This makes it difficult for OVS users to monitor packet drop in an OVS
instance and to alert a management system in case of a unexpected increase
of such drops. Also OVS trouble-shooters face difficulties in analysing
packet drops.
With this patch we implement following changes to address the issues
mentioned above.
1. Identify and account all the silent packet drop scenarios
2. Display these drops in ovs-appctl coverage/show
A detailed presentation on this was presented at OvS conference 2017 and
link for the corresponding presentation is available at:
https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329
Co-authored-by: Rohith Basavaraja
Co-authored-by: Keshav Gupta
Signed-off-by: Anju Thomas
Signed-off-by: Rohith Basavaraja
Signed-off-by: Keshav Gupta
---
datapath/linux/compat/include/linux/openvswitch.h | 1 +
lib/dpif-netdev.c | 39 ++-
lib/dpif.c| 7 ++
lib/dpif.h| 3 +
lib/netdev-dpdk.c | 4 ++
lib/odp-execute.c | 81 +++
lib/odp-execute.h | 2 +
lib/odp-util.c| 10 ++-
ofproto/ofproto-dpif-ipfix.c | 1 +
ofproto/ofproto-dpif-sflow.c | 1 +
ofproto/ofproto-dpif-xlate.c | 31 +
ofproto/ofproto-dpif-xlate.h | 4 ++
ofproto/ofproto-dpif.c| 8 +++
ofproto/ofproto-dpif.h| 8 ++-
tests/automake.mk | 3 +-
tests/dpif-netdev.at | 10 +++
tests/ofproto-dpif.at | 4 +-
tests/testsuite.at| 1 +
tests/tunnel-push-pop.at | 28 +++-
tests/tunnel.at | 14 +++-
20 files changed, 248 insertions(+), 12 deletions(-)
diff --git a/datapath/linux/compat/include/linux/openvswitch.h
b/datapath/linux/compat/include/linux/openvswitch.h
index 9b087f1..92db378 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -938,6 +938,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_POP_NSH, /* No argument. */
OVS_ACTION_ATTR_METER,/* u32 meter number. */
OVS_ACTION_ATTR_CLONE,/* Nested OVS_CLONE_ATTR_*. */
+ OVS_ACTION_ATTR_DROP,
#ifndef __KERNEL__
OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0f57e3f..c726463 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -77,6 +77,7 @@
#include "unixctl.h"
#include "util.h"
#include "uuid.h"
+#include "ofproto/ofproto-dpif-xlate.h"
VLOG_DEFINE_THIS_MODULE(dpif_netdev);
@@ -100,6 +101,17 @@ enum { MAX_METERS = 65536 };/* Maximum number of
meters. */
enum { MAX_BANDS = 8 }; /* Maximum number of bands / meter. */
enum { N_METER_LOCKS = 64 };/* Maximum number of meters. */
+COVERAGE_DEFINE(datapath_drop_meter);
+COVERAGE_DEFINE(datapath_drop_upcall_error);
+COVERAGE_DEFINE(datapath_drop_lock_error);
+COVERAGE_DEFINE(datapath_drop_userspace_action_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_push_error);
+COVERAGE_DEFINE(datapath_drop_tunnel_pop_error);
+COVERAGE_DEFINE(datapath_drop_recirc_error);
+COVERAGE_DEFINE(datapath_drop_invalid_port);
+COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
+COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -5643,7 +5655,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct
dp_packet_batch *packets_,
band =
Re: [ovs-dev] [ovs-dev,v6] Improved Packet Drop Statistics in OVS
Thanks Ben and Ilya.
I will incorporate the other changes.
I just have below comments.
>> @@ -2427,8 +2474,13 @@ odp_actions_from_string(const char *s, const struct
>> simap *port_names,
>> struct ofpbuf *actions)
>> {
>> size_t old_size;
>> +struct ovs_action_drop drop_action;
>>
>> -if (!strcasecmp(s, "drop")) {
>> +if ((!strcasecmp(s, "drop") ||
>> +!strcasecmp(s, "drop:pipeline-drop"))) {
>
> Why you're adding parse only for this type of drop action ?
> The rest are all below error scenarios which we hit during processing
> and hence I thought perhaps may not make sense as an input?? . Do you think
> it make sense to do that ?
I think, Ben already answered this questin.
IMHO, that parsing of only one type makes no sence. There should be all
or nothing. As we think that OVS should be able to parse flows it generates,
there should be all.
In that case, i propose removing this to ensure no subtypes are parsed.
Can we agree on this ? In case in future there is a use case,we can revisit
this .
>
> Above counters could be defined inside odp-execute.c. You already have
> 2 other counters there. In this case there will be no need to introduce
> additional 'dp_update_drop_action_counter_cb' callback and pass it around.
> All the required logic will also be moved to odp-execute.c.
>
> But I want the callback to be NULL when called from
> dpif_execute_with_help
Why? This seems strange to count drops only for actions that do not need
help from the dpif layer.
For now we would want to add only for drop in actual dpdk datapath. We
will revist this
Later
Regards
Anju
-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Wednesday, January 23, 2019 8:27 PM
To: Anju Thomas ; d...@openvswitch.org
Cc: Keshav Gupta ; Stokes, Ian ;
Ben Pfaff
Subject: Re: [ovs-dev,v6] Improved Packet Drop Statistics in OVS
On 23.01.2019 16:53, Anju Thomas wrote:
> Hi Ilya ,
> Some queries w.r.t your comments .
>
> Regards
> Anju
>
> -Original Message-
> From: Ilya Maximets [mailto:i.maxim...@samsung.com]
> Sent: Friday, January 18, 2019 5:33 PM
> To: Anju Thomas ; d...@openvswitch.org
> Cc: Keshav Gupta ; Stokes, Ian ;
> Ben Pfaff
> Subject: Re: [ovs-dev,v6] Improved Packet Drop Statistics in OVS
>
> Hi.
> Thanks for working on this.
> Some general comments:
>
> 1. This patch consists of two separate features:
>- Reporting the drop reason in flow dumps.
>- Counters for different drop types.
>I still think that there should be patch-set of two separate patches.
>This will simplify the review and allow to apply them separately/speed
>up accepting.
>
> 2. There are some issues with the patch formatting. The commit message is
>indented like in the output of 'git show' command. So, it's double
>shifted after applying in git. Please remove the additional indentation.
>You probably should start using 'git format-patch'.
>
>Second issue:
>
>Applying: Improved Packet Drop Statistics in OVS
>.git/rebase-apply/patch:415: new blank line at EOF.
>+
>warning: 1 line adds whitespace errors.
>
>One more thing is that for better readability it's a common practice to
>limit the width of lines in commit-message to 72 characters. For example,
>you may strip not important fields of the dump-flows output. Like this:
>
>recirc_id(0),in_port(5),<...>, actions:drop:recursion too deep
>
>
> Other comments inline.
>
> I use
> a)git format-patch -1 -s
> b) ./utilities/checkpatch.py -1
>
> Are these the correct parameters that I should be using ?
Commands looks fine. If the patch looks same in your git repo, I'd suggest you
to re-format the patch by hands.
Also, I just spotted that you have a lot of duplicated sign-offs in the patch.
>
> Best regards, Ilya Maximets.
>
> On 17.01.2019 7:49, Anju Thomas wrote:
>> Currently OVS maintains explicit packet drop/error counters only on port
>> level. Packets that are dropped as part of normal OpenFlow processing are
>> counted in flow stats of “drop” flows or as table misses in table stats.
>> These can only be interpreted by controllers that know the semantics of
>> the configured OpenFlow pipeline. Without that knowledge, it is
>> impossible
>> for an OVS user to obtain e.g. the total number of packets dropped due to
>> OpenFlow rules.
>>
>> Furthermore, there are numerous other reasons for which packets can be
>> dropped by OVS slow path that are not relate
Re: [ovs-dev] [ovs-dev,v6] Improved Packet Drop Statistics in OVS
Hi Ilya , Some queries w.r.t your comments . Regards Anju -Original Message- From: Ilya Maximets [mailto:i.maxim...@samsung.com] Sent: Friday, January 18, 2019 5:33 PM To: Anju Thomas ; d...@openvswitch.org Cc: Keshav Gupta ; Stokes, Ian ; Ben Pfaff Subject: Re: [ovs-dev,v6] Improved Packet Drop Statistics in OVS Hi. Thanks for working on this. Some general comments: 1. This patch consists of two separate features: - Reporting the drop reason in flow dumps. - Counters for different drop types. I still think that there should be patch-set of two separate patches. This will simplify the review and allow to apply them separately/speed up accepting. 2. There are some issues with the patch formatting. The commit message is indented like in the output of 'git show' command. So, it's double shifted after applying in git. Please remove the additional indentation. You probably should start using 'git format-patch'. Second issue: Applying: Improved Packet Drop Statistics in OVS .git/rebase-apply/patch:415: new blank line at EOF. + warning: 1 line adds whitespace errors. One more thing is that for better readability it's a common practice to limit the width of lines in commit-message to 72 characters. For example, you may strip not important fields of the dump-flows output. Like this: recirc_id(0),in_port(5),<...>, actions:drop:recursion too deep Other comments inline. I use a)git format-patch -1 -s b) ./utilities/checkpatch.py -1 Are these the correct parameters that I should be using ? Best regards, Ilya Maximets. On 17.01.2019 7:49, Anju Thomas wrote: > Currently OVS maintains explicit packet drop/error counters only on port > level. Packets that are dropped as part of normal OpenFlow processing are > counted in flow stats of “drop” flows or as table misses in table stats. > These can only be interpreted by controllers that know the semantics of > the configured OpenFlow pipeline. Without that knowledge, it is impossible > for an OVS user to obtain e.g. the total number of packets dropped due to > OpenFlow rules. > > Furthermore, there are numerous other reasons for which packets can be > dropped by OVS slow path that are not related to the OpenFlow pipeline. > The generated datapath flow entries include a drop action to avoid further > expensive upcalls to the slow path, but subsequent packets dropped by the > datapath are not accounted anywhere. > > Finally, the datapath itself drops packets in certain error situations. > Also, these drops are today not accounted for. > > This makes it difficult for OVS users to monitor packet drop in an OVS > instance and to alert a management system in case of a unexpected increase > of such drops. Also OVS trouble-shooters face difficulties in analysing > packet drops. > > With this patch we implement following changes to address the issues > mentioned above. > > 1. Account and categorize all the packet drops in OVS. > 2. Account & classify “drop” action packet drops according to the drop >reason. > 3. Identify and account all the silent packet drop scenarios. > 4. Display these drops in ovs-appctl coverage/show > 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows >to print drop reason along with drop action > > A detailed presentation on this was presented at OvS conference 2017 and > link for the corresponding presentation is available at: > > https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 > > Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows > displaying drop reason along with drop action. > > The idea is to use the coverage infrastructure to maintain the drops > > $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev > flow-dump from pmd on cpu core: 0 > > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), > packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep > > $ ovs-appctl dpif/dump-flows br-int > > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), > packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), > packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep > >In subsequent commits, we are planning to see if we can use this > infrastructure to create a >wrapper to clear and display counters as well. > > Co-authored-by: Rohith Basavaraja > Co-authored-by: Keshav Gupta > Signed-off-by: Anju Thomas > Signed-off-by:
Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS
Hi Ben, Thanks for reply. I just have one doubt . Reply inline Regards Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Wednesday, January 23, 2019 5:57 AM To: Anju Thomas Cc: d...@openvswitch.org; Keshav Gupta Subject: Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS On Mon, Jan 21, 2019 at 07:01:33AM +, Anju Thomas wrote: > On Thu, Jan 17, 2019 at 04:49:20AM +0000, Anju Thomas wrote: > > Currently OVS maintains explicit packet drop/error counters only on port > > level. Packets that are dropped as part of normal OpenFlow processing > > are > > counted in flow stats of “drop” flows or as table misses in table stats. > > These can only be interpreted by controllers that know the semantics of > > the configured OpenFlow pipeline. Without that knowledge, it is > > impossible > > for an OVS user to obtain e.g. the total number of packets dropped due > > to > > OpenFlow rules. > > Thanks for the patch! I agree with your motivations--it is useful to > understand why packets are dropped. I have some comments to add to Ilya's. > > It looks like the drop actions that this formats in > format_odp_drop_action() can't necessarily be parsed by > odp_actions_from_string(). Usually we expect this. (Probably the syntax > should be adjusted to make parsing more straightforward.) > odp_actions_from string is used to convert string to action for parsing .But > we are using the drop action reason only fo display. We do not want to > provide an option for its parsing. Is my understanding correct. Why not? Ordinarily, anything that can be output can be input. It is useful for testing to be able to input datapath actions. Agreed. Ideally yes. But in this case the error are set due to some error during translation like *bridge not found *too many resubmits *recursion too deep *too many resubmits *sack too deeo *no recirculation context *too many mpls labels * invalid tunnel metadata * unsupported packet type * ecn mismatch at tunnel decapsulation * forwarding disabled * pipeline drop Except pipeline drop I don’t understand how it will make sense to inject others as input . Can you explain if I am missing something ? > It looks like xlate_error maps one-to-one to drop reasons (except why is > XLATE_FORWARDING_DISABLED mapped to OVS_DROP_REASON_MAX?), so do we really > want different enumerations? Mapping back and forth is a bit of a slog, and > there's already a way to translate xlate_errors to strings. > > .We just kept it separate for better code readability and also that > XLATE_** error were more in the ofproto layer and we wanted something in the > datapath . If you believe that that is not justified compared to the slogging > then I will modify this . What do you say ? I'd leave them the same for now, until there's a need for them to be different. Sure. I will keep them same error viz XLATE_** codes. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Thanks Ben. Regards Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Wednesday, January 23, 2019 5:24 AM To: Anju Thomas Cc: Stokes, Ian ; Lam, Tiago ; d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action OK. I applied the first part to all relevant branches. On Mon, Jan 21, 2019 at 04:06:26AM +, Anju Thomas wrote: > Hi Ben/Ian, > I understand that the second part of the patch needs more review. But > do you think we can atleast merge the first part of this viz > https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346649.html so that > to crash and more importantly silent leaks . We can widely review the other > part of the code as it might require more indepth testing and review. > > What are your suggestions ? > > Regards > Anju > > -Original Message- > From: Ben Pfaff [mailto:b...@ovn.org] > Sent: Thursday, January 17, 2019 11:39 PM > To: Stokes, Ian > Cc: Lam, Tiago ; Anju Thomas > ; d...@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push > action > > On Wed, Jan 16, 2019 at 11:38:38AM +, Stokes, Ian wrote: > > > On 16/01/2019 09:30, Anju Thomas wrote: > > > > > > > > Hi Folks, > > > > > > > > Are these changes planned to be merged as well? > > > > > > > > Regards > > > > Anju > > > > > > Hi Anju, > > > > > > Unfortunately, no. An RFC based on the below was proposed to the > > > mailing list here [1], but no discussion / comments happened after > > > that. Further discussion and testing would be needed to move this > > > forward... > > > > > > > I wasn't aware of this tbh. It seems like a bug fix so I believe it would > > be eligible to be merged for the 2.11 release a(and possibly backported > > also). > > > > As it manipulates the dp packets functionality it would need wider testing > > and discussion among the community. > > > > It won't make it upstream this week before we cut for 2.11 I would think > > but there is 4 weeks allowed until release where bug fixes are allowed. If > > further work continues it could be merged within that window or afterwards > > and backported? > > Bug fixes are welcomed at any time, pre- or post-release. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS
Hi Ben, Thank you for the comments. Replies inline Regards Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Friday, January 18, 2019 11:45 PM To: Anju Thomas Cc: d...@openvswitch.org; Keshav Gupta Subject: Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS On Thu, Jan 17, 2019 at 04:49:20AM +, Anju Thomas wrote: > Currently OVS maintains explicit packet drop/error counters only on port > level. Packets that are dropped as part of normal OpenFlow processing are > counted in flow stats of “drop” flows or as table misses in table stats. > These can only be interpreted by controllers that know the semantics of > the configured OpenFlow pipeline. Without that knowledge, it is impossible > for an OVS user to obtain e.g. the total number of packets dropped due to > OpenFlow rules. Thanks for the patch! I agree with your motivations--it is useful to understand why packets are dropped. I have some comments to add to Ilya's. It looks like the drop actions that this formats in format_odp_drop_action() can't necessarily be parsed by odp_actions_from_string(). Usually we expect this. (Probably the syntax should be adjusted to make parsing more straightforward.) odp_actions_from string is used to convert string to action for parsing .But we are using the drop action reason only fo display. We do not want to provide an option for its parsing. Is my understanding correct. It looks like xlate_error maps one-to-one to drop reasons (except why is XLATE_FORWARDING_DISABLED mapped to OVS_DROP_REASON_MAX?), so do we really want different enumerations? Mapping back and forth is a bit of a slog, and there's already a way to translate xlate_errors to strings. .We just kept it separate for better code readability and also that XLATE_** error were more in the ofproto layer and we wanted something in the datapath . If you believe that that is not justified compared to the slogging then I will modify this . What do you say ? This exports coverage_mutex but I don't see why since nothing new uses it. Actually I think all the changes to coverage.[ch] are unneeded. You are right. I will change this This adds and removes a number of blank lines, I don't see the value in that. Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev,v6] Improved Packet Drop Statistics in OVS
Thanks Ilya. I will work on these as well. -Original Message- From: Ilya Maximets [mailto:i.maxim...@samsung.com] Sent: Friday, January 18, 2019 5:33 PM To: Anju Thomas ; d...@openvswitch.org Cc: Keshav Gupta ; Stokes, Ian ; Ben Pfaff Subject: Re: [ovs-dev,v6] Improved Packet Drop Statistics in OVS Hi. Thanks for working on this. Some general comments: 1. This patch consists of two separate features: - Reporting the drop reason in flow dumps. - Counters for different drop types. I still think that there should be patch-set of two separate patches. This will simplify the review and allow to apply them separately/speed up accepting. 2. There are some issues with the patch formatting. The commit message is indented like in the output of 'git show' command. So, it's double shifted after applying in git. Please remove the additional indentation. You probably should start using 'git format-patch'. Second issue: Applying: Improved Packet Drop Statistics in OVS .git/rebase-apply/patch:415: new blank line at EOF. + warning: 1 line adds whitespace errors. One more thing is that for better readability it's a common practice to limit the width of lines in commit-message to 72 characters. For example, you may strip not important fields of the dump-flows output. Like this: recirc_id(0),in_port(5),<...>, actions:drop:recursion too deep Other comments inline. Best regards, Ilya Maximets. On 17.01.2019 7:49, Anju Thomas wrote: > Currently OVS maintains explicit packet drop/error counters only on port > level. Packets that are dropped as part of normal OpenFlow processing are > counted in flow stats of “drop” flows or as table misses in table stats. > These can only be interpreted by controllers that know the semantics of > the configured OpenFlow pipeline. Without that knowledge, it is impossible > for an OVS user to obtain e.g. the total number of packets dropped due to > OpenFlow rules. > > Furthermore, there are numerous other reasons for which packets can be > dropped by OVS slow path that are not related to the OpenFlow pipeline. > The generated datapath flow entries include a drop action to avoid further > expensive upcalls to the slow path, but subsequent packets dropped by the > datapath are not accounted anywhere. > > Finally, the datapath itself drops packets in certain error situations. > Also, these drops are today not accounted for. > > This makes it difficult for OVS users to monitor packet drop in an OVS > instance and to alert a management system in case of a unexpected increase > of such drops. Also OVS trouble-shooters face difficulties in analysing > packet drops. > > With this patch we implement following changes to address the issues > mentioned above. > > 1. Account and categorize all the packet drops in OVS. > 2. Account & classify “drop” action packet drops according to the drop >reason. > 3. Identify and account all the silent packet drop scenarios. > 4. Display these drops in ovs-appctl coverage/show > 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows >to print drop reason along with drop action > > A detailed presentation on this was presented at OvS conference 2017 and > link for the corresponding presentation is available at: > > https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 > > Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows > displaying drop reason along with drop action. > > The idea is to use the coverage infrastructure to maintain the drops > > $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev > flow-dump from pmd on cpu core: 0 > > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), > packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep > > $ ovs-appctl dpif/dump-flows br-int > > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), > packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), > packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep > >In subsequent commits, we are planning to see if we can use this > infrastructure to create a >wrapper to clear and display counters as well. > > Co-authored-by: Rohith Basavaraja > Co-authored-by: Keshav Gupta > Signed-off-by: Anju Thomas > Signed-off-by: Rohith Basavaraja > Signed-off-by: Keshav Gupta > Signed-off-by: Anju Thomas > Signed-off-by: Rohith Basavaraja &g
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Ben/Ian, I understand that the second part of the patch needs more review. But do you think we can atleast merge the first part of this viz https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346649.html so that to crash and more importantly silent leaks . We can widely review the other part of the code as it might require more indepth testing and review. What are your suggestions ? Regards Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Thursday, January 17, 2019 11:39 PM To: Stokes, Ian Cc: Lam, Tiago ; Anju Thomas ; d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action On Wed, Jan 16, 2019 at 11:38:38AM +, Stokes, Ian wrote: > > On 16/01/2019 09:30, Anju Thomas wrote: > > > > > > Hi Folks, > > > > > > Are these changes planned to be merged as well? > > > > > > Regards > > > Anju > > > > Hi Anju, > > > > Unfortunately, no. An RFC based on the below was proposed to the > > mailing list here [1], but no discussion / comments happened after > > that. Further discussion and testing would be needed to move this forward... > > > > I wasn't aware of this tbh. It seems like a bug fix so I believe it would be > eligible to be merged for the 2.11 release a(and possibly backported also). > > As it manipulates the dp packets functionality it would need wider testing > and discussion among the community. > > It won't make it upstream this week before we cut for 2.11 I would think but > there is 4 weeks allowed until release where bug fixes are allowed. If > further work continues it could be merged within that window or afterwards > and backported? Bug fixes are welcomed at any time, pre- or post-release. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS
Thank you Ben, Regards Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Friday, January 18, 2019 10:35 AM To: Anju Thomas Cc: d...@openvswitch.org; Rohith Basavaraja ; Keshav Gupta Subject: Re: [PATCH v6] Improved Packet Drop Statistics in OVS Yes. Thank you. I'll review it tomorrow. On Fri, Jan 18, 2019 at 04:54:51AM +0000, Anju Thomas wrote: > Hi Ben, > Were you able to apply this patch successfully? > Regards > Anju > > -Original Message----- > From: Anju Thomas [mailto:anju.tho...@ericsson.com] > Sent: Thursday, January 17, 2019 10:19 AM > To: d...@openvswitch.org > Cc: Anju Thomas ; Rohith Basavaraja > ; Keshav Gupta > Subject: [PATCH v6] Improved Packet Drop Statistics in OVS > > Currently OVS maintains explicit packet drop/error counters only on port > level. Packets that are dropped as part of normal OpenFlow processing are > counted in flow stats of “drop” flows or as table misses in table stats. > These can only be interpreted by controllers that know the semantics of > the configured OpenFlow pipeline. Without that knowledge, it is impossible > for an OVS user to obtain e.g. the total number of packets dropped due to > OpenFlow rules. > > Furthermore, there are numerous other reasons for which packets can be > dropped by OVS slow path that are not related to the OpenFlow pipeline. > The generated datapath flow entries include a drop action to avoid further > expensive upcalls to the slow path, but subsequent packets dropped by the > datapath are not accounted anywhere. > > Finally, the datapath itself drops packets in certain error situations. > Also, these drops are today not accounted for. > > This makes it difficult for OVS users to monitor packet drop in an OVS > instance and to alert a management system in case of a unexpected increase > of such drops. Also OVS trouble-shooters face difficulties in analysing > packet drops. > > With this patch we implement following changes to address the issues > mentioned above. > > 1. Account and categorize all the packet drops in OVS. > 2. Account & classify “drop” action packet drops according to the drop >reason. > 3. Identify and account all the silent packet drop scenarios. > 4. Display these drops in ovs-appctl coverage/show > 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows >to print drop reason along with drop action > > A detailed presentation on this was presented at OvS conference 2017 and > link for the corresponding presentation is available at: > > https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 > > Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows > displaying drop reason along with drop action. > > The idea is to use the coverage infrastructure to maintain the drops > > $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev > flow-dump from pmd on cpu core: 0 > > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), > packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep > > $ ovs-appctl dpif/dump-flows br-int > > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), > packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep > recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), > packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep > >In subsequent commits, we are planning to see if we can use this > infrastructure to create a >wrapper to clear and display counters as well. > > Co-authored-by: Rohith Basavaraja > Co-authored-by: Keshav Gupta > Signed-off-by: Anju Thomas > Signed-off-by: Rohith Basavaraja > Signed-off-by: Keshav Gupta > --- > datapath/linux/compat/include/linux/openvswitch.h | 51 ++ > lib/coverage.c| 6 +- > lib/coverage.h| 3 +- > lib/dpif-netdev.c | 105 +++- > lib/dpif.c| 10 +- > lib/dpif.h| 3 + > lib/netdev-dpdk.c | 4 + > lib/odp-execute.c | 50 -- > lib/odp-execute.h | 10 +- > lib/odp-util.c| 54 +- > ofproto/ofproto-dpif-ipfix.c | 1 + > ofproto/ofproto-dpif-sflow.c | 1 + > ofproto/ofproto-dpif-upcall.c
Re: [ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS
Hi Ben, Were you able to apply this patch successfully? Regards Anju -Original Message- From: Anju Thomas [mailto:anju.tho...@ericsson.com] Sent: Thursday, January 17, 2019 10:19 AM To: d...@openvswitch.org Cc: Anju Thomas ; Rohith Basavaraja ; Keshav Gupta Subject: [PATCH v6] Improved Packet Drop Statistics in OVS Currently OVS maintains explicit packet drop/error counters only on port level. Packets that are dropped as part of normal OpenFlow processing are counted in flow stats of “drop” flows or as table misses in table stats. These can only be interpreted by controllers that know the semantics of the configured OpenFlow pipeline. Without that knowledge, it is impossible for an OVS user to obtain e.g. the total number of packets dropped due to OpenFlow rules. Furthermore, there are numerous other reasons for which packets can be dropped by OVS slow path that are not related to the OpenFlow pipeline. The generated datapath flow entries include a drop action to avoid further expensive upcalls to the slow path, but subsequent packets dropped by the datapath are not accounted anywhere. Finally, the datapath itself drops packets in certain error situations. Also, these drops are today not accounted for. This makes it difficult for OVS users to monitor packet drop in an OVS instance and to alert a management system in case of a unexpected increase of such drops. Also OVS trouble-shooters face difficulties in analysing packet drops. With this patch we implement following changes to address the issues mentioned above. 1. Account and categorize all the packet drops in OVS. 2. Account & classify “drop” action packet drops according to the drop reason. 3. Identify and account all the silent packet drop scenarios. 4. Display these drops in ovs-appctl coverage/show 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows to print drop reason along with drop action A detailed presentation on this was presented at OvS conference 2017 and link for the corresponding presentation is available at: https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows displaying drop reason along with drop action. The idea is to use the coverage infrastructure to maintain the drops $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev flow-dump from pmd on cpu core: 0 recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep $ ovs-appctl dpif/dump-flows br-int recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep In subsequent commits, we are planning to see if we can use this infrastructure to create a wrapper to clear and display counters as well. Co-authored-by: Rohith Basavaraja Co-authored-by: Keshav Gupta Signed-off-by: Anju Thomas Signed-off-by: Rohith Basavaraja Signed-off-by: Keshav Gupta --- datapath/linux/compat/include/linux/openvswitch.h | 51 ++ lib/coverage.c| 6 +- lib/coverage.h| 3 +- lib/dpif-netdev.c | 105 +++- lib/dpif.c| 10 +- lib/dpif.h| 3 + lib/netdev-dpdk.c | 4 + lib/odp-execute.c | 50 -- lib/odp-execute.h | 10 +- lib/odp-util.c| 54 +- ofproto/ofproto-dpif-ipfix.c | 1 + ofproto/ofproto-dpif-sflow.c | 1 + ofproto/ofproto-dpif-upcall.c | 3 +- ofproto/ofproto-dpif-xlate.c | 64 +++ ofproto/ofproto-dpif-xlate.h | 4 + ofproto/ofproto-dpif.c| 11 +- ofproto/ofproto-dpif.h| 8 +- tests/automake.mk | 3 +- tests/bundle.at | 2 +- tests/classifier.at | 10 +- tests/dpif-netdev.at | 17 +- tests/drop-stats.at | 197 ++ tests/ofproto-dpif.at | 118 ++--- tests/ovs-ofctl.at| 2 +- tests/packet-type-aware.at| 17 +- tests/testsuite.at| 1 + tests/tunnel-pus
[ovs-dev] [PATCH v6] Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level. Packets that are dropped as part of normal OpenFlow processing are counted in flow stats of “drop” flows or as table misses in table stats. These can only be interpreted by controllers that know the semantics of the configured OpenFlow pipeline. Without that knowledge, it is impossible for an OVS user to obtain e.g. the total number of packets dropped due to OpenFlow rules. Furthermore, there are numerous other reasons for which packets can be dropped by OVS slow path that are not related to the OpenFlow pipeline. The generated datapath flow entries include a drop action to avoid further expensive upcalls to the slow path, but subsequent packets dropped by the datapath are not accounted anywhere. Finally, the datapath itself drops packets in certain error situations. Also, these drops are today not accounted for. This makes it difficult for OVS users to monitor packet drop in an OVS instance and to alert a management system in case of a unexpected increase of such drops. Also OVS trouble-shooters face difficulties in analysing packet drops. With this patch we implement following changes to address the issues mentioned above. 1. Account and categorize all the packet drops in OVS. 2. Account & classify “drop” action packet drops according to the drop reason. 3. Identify and account all the silent packet drop scenarios. 4. Display these drops in ovs-appctl coverage/show 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows to print drop reason along with drop action A detailed presentation on this was presented at OvS conference 2017 and link for the corresponding presentation is available at: https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows displaying drop reason along with drop action. The idea is to use the coverage infrastructure to maintain the drops $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev flow-dump from pmd on cpu core: 0 recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep $ ovs-appctl dpif/dump-flows br-int recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep In subsequent commits, we are planning to see if we can use this infrastructure to create a wrapper to clear and display counters as well. Co-authored-by: Rohith Basavaraja Co-authored-by: Keshav Gupta Signed-off-by: Anju Thomas Signed-off-by: Rohith Basavaraja Signed-off-by: Keshav Gupta --- datapath/linux/compat/include/linux/openvswitch.h | 51 ++ lib/coverage.c| 6 +- lib/coverage.h| 3 +- lib/dpif-netdev.c | 105 +++- lib/dpif.c| 10 +- lib/dpif.h| 3 + lib/netdev-dpdk.c | 4 + lib/odp-execute.c | 50 -- lib/odp-execute.h | 10 +- lib/odp-util.c| 54 +- ofproto/ofproto-dpif-ipfix.c | 1 + ofproto/ofproto-dpif-sflow.c | 1 + ofproto/ofproto-dpif-upcall.c | 3 +- ofproto/ofproto-dpif-xlate.c | 64 +++ ofproto/ofproto-dpif-xlate.h | 4 + ofproto/ofproto-dpif.c| 11 +- ofproto/ofproto-dpif.h| 8 +- tests/automake.mk | 3 +- tests/bundle.at | 2 +- tests/classifier.at | 10 +- tests/dpif-netdev.at | 17 +- tests/drop-stats.at | 197 ++ tests/ofproto-dpif.at | 118 ++--- tests/ovs-ofctl.at| 2 +- tests/packet-type-aware.at| 17 +- tests/testsuite.at| 1 + tests/tunnel-push-pop.at | 26 ++- tests/tunnel.at | 21 ++- 28 files changed, 695 insertions(+), 107 deletions(-) create mode 100644 tests/drop-stats.at diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index 9b087f1..b66b46
Re: [ovs-dev] [PATCH v5] :Improved Packet Drop Statistics in OVS
Sure Ben. I will send the patch right away. Regards Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Thursday, January 17, 2019 2:18 AM To: Anju Thomas Cc: d...@openvswitch.org; Keshav Gupta Subject: Re: [ovs-dev] [PATCH v5] :Improved Packet Drop Statistics in OVS On Mon, Jan 07, 2019 at 09:59:24AM +, Anju Thomas wrote: > Currently OVS maintains explicit packet drop/error counters only on port > level. Packets that are dropped as part of normal OpenFlow processing are > counted in flow stats of “drop” flows or as table misses in table stats. > These can only be interpreted by controllers that know the semantics of > the configured OpenFlow pipeline. Without that knowledge, it is impossible > for an OVS user to obtain e.g. the total number of packets dropped due to > OpenFlow rules. Thanks for the patch! I see one application failure: Applying: Currently OVS maintains explicit packet drop/error counters only on port /home/blp/nicira/ovs.git/worktrees/ovs/rebase-apply/patch:416: new blank line at EOF. + error: patch failed: tests/odp.at:100 error: tests/odp.at: patch does not apply Patch failed at 0001 Currently OVS maintains explicit packet drop/error counters only on port Can you resubmit after rebasing against current master? Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Ian, Thanks for getting back . I agree that the patch Tiago has posted in the latest needs more testing but this is more sort of an improvement. Can we merge the first part of the problem as addressed in https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346649.html This will atleast prevent silent memory leaks and irrelevant tunnel header pushes . Regards Anju -Original Message- From: Stokes, Ian [mailto:ian.sto...@intel.com] Sent: Wednesday, January 16, 2019 5:09 PM To: Lam, Tiago ; Anju Thomas ; Ben Pfaff Cc: d...@openvswitch.org Subject: RE: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action > On 16/01/2019 09:30, Anju Thomas wrote: > > > > Hi Folks, > > > > Are these changes planned to be merged as well? > > > > Regards > > Anju > > Hi Anju, > > Unfortunately, no. An RFC based on the below was proposed to the > mailing list here [1], but no discussion / comments happened after > that. Further discussion and testing would be needed to move this forward... > I wasn't aware of this tbh. It seems like a bug fix so I believe it would be eligible to be merged for the 2.11 release a(and possibly backported also). As it manipulates the dp packets functionality it would need wider testing and discussion among the community. It won't make it upstream this week before we cut for 2.11 I would think but there is 4 weeks allowed until release where bug fixes are allowed. If further work continues it could be merged within that window or afterwards and backported? Ian ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5] :Improved Packet Drop Statistics in OVS
Hi folks, Do these changes look ok? Regards & thanks Anju -Original Message- From: Anju Thomas [mailto:anju.tho...@ericsson.com] Sent: Monday, January 07, 2019 3:29 PM To: d...@openvswitch.org Cc: Anju Thomas ; Rohith Basavaraja ; Keshav Gupta Subject: [PATCH v5] :Improved Packet Drop Statistics in OVS Currently OVS maintains explicit packet drop/error counters only on port level. Packets that are dropped as part of normal OpenFlow processing are counted in flow stats of “drop” flows or as table misses in table stats. These can only be interpreted by controllers that know the semantics of the configured OpenFlow pipeline. Without that knowledge, it is impossible for an OVS user to obtain e.g. the total number of packets dropped due to OpenFlow rules. Furthermore, there are numerous other reasons for which packets can be dropped by OVS slow path that are not related to the OpenFlow pipeline. The generated datapath flow entries include a drop action to avoid further expensive upcalls to the slow path, but subsequent packets dropped by the datapath are not accounted anywhere. Finally, the datapath itself drops packets in certain error situations. Also, these drops are today not accounted for. This makes it difficult for OVS users to monitor packet drop in an OVS instance and to alert a management system in case of a unexpected increase of such drops. Also OVS trouble-shooters face difficulties in analysing packet drops. With this patch we implement following changes to address the issues mentioned above. 1. Account and categorize all the packet drops in OVS. 2. Account & classify “drop” action packet drops according to the drop reason. 3. Identify and account all the silent packet drop scenarios. 4. Display these drops in ovs-appctl coverage/show 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows to print drop reason along with drop action A detailed presentation on this was presented at OvS conference 2017 and link for the corresponding presentation is available at: https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows displaying drop reason along with drop action. The idea is to use the coverage infrastructure to maintain the drops $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev flow-dump from pmd on cpu core: 0 recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep $ ovs-appctl dpif/dump-flows br-int recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep In subsequent commits, we are planning to see if we can use this infrastructure to create a wrapper to clear and display counters as well. Co-authored-by: Rohith Basavaraja Co-authored-by: Keshav Gupta Signed-off-by: Anju Thomas Signed-off-by: Rohith Basavaraja Signed-off-by: Keshav Gupta --- datapath/linux/compat/include/linux/openvswitch.h | 51 ++ lib/coverage.c| 6 +- lib/coverage.h| 3 +- lib/dpif-netdev.c | 105 +++- lib/dpif.c| 10 +- lib/dpif.h| 3 + lib/netdev-dpdk.c | 4 + lib/odp-execute.c | 50 -- lib/odp-execute.h | 10 +- lib/odp-util.c| 54 +- ofproto/ofproto-dpif-ipfix.c | 1 + ofproto/ofproto-dpif-sflow.c | 1 + ofproto/ofproto-dpif-upcall.c | 3 +- ofproto/ofproto-dpif-xlate.c | 64 +++ ofproto/ofproto-dpif-xlate.h | 4 + ofproto/ofproto-dpif.c| 11 +- ofproto/ofproto-dpif.h| 8 +- tests/automake.mk | 3 +- tests/bundle.at | 2 +- tests/classifier.at | 10 +- tests/dpif-netdev.at | 17 +- tests/drop-stats.at | 197 ++ tests/odp.at | 6 +- tests/ofproto-dpif.at | 118 ++--- tests/ovs-ofctl.at| 2 +- tests/packet-type-aware.at| 17 +- tests/
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Folks,
Are these changes planned to be merged as well?
Regards
Anju
-Original Message-
From: Lam, Tiago [mailto:tiago@intel.com]
Sent: Monday, July 02, 2018 11:27 PM
To: Anju Thomas ; Ben Pfaff
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
On 25/06/2018 13:15, Anju Thomas wrote:
> Hi Ben,
>
> We are facing multiple such crashes on different computes in our deployments.
> Seems to be a pretty common problem in our setup. As you suggested, it would
> be good if we can make the below changes as well.How do you suggest we
> move forward regarding the approaches?
>
> Regards & thanks
> Anju
>
Hi Anju,
I agree that this patch should go in irrespective of the result of this
discussion. It is a bug nonetheless (which in this case, as you explained, is
triggering a crash because of dp_packet_resize__() calls on DPBUF_DPDK packets).
The approach of completely refactoring dp_packet to properly deal with errors,
even though it is the safest option, seems like it would require a fair amount
of change compared to what we actually need to cover, which is "resize
operations on DPBUF_DPDK packets". Resize operations on any other packets (!=
DPBUF_DPDK) wouldn't require any type of change:
if memory is not available, xmalloc() will deal with that by aborting.
Thus, any packet created with dp_packet_new() / dp_packet_init() /
dp_packet_use_stub(), or even cloned (a DPBUF_DPDK is cloned into a
DPBUF_MALLOC, for example), won't have these limitations.
So, going back to the approaches you described before, Anju, my preference
would be to go towards approach 2 (or better yet, an alternate version of 2).
In this version, dp_packet_put_uninit() and
dp_packet_push_uninit() wouldn't call neither
dp_packet_prealloc_tailroom() nor dp_packet_prealloc_headroom() for DPBUF_DPDK
packets. Instead, they would check if there's enough space beforehand, and if
not return NULL, instead of the normal pointer to the data. Otherwise the
operations would continue as expected.
This means we would need to deal with the NULL case only where we may be using
DPBUF_DPDK packets (all the other cases remain unaffected as NULL won't be
possible there). The cases I've identified it happens is where headers are
being pushed (which seems to be what's causing your crashes as well):
- In eth_push_vlan(), push_eth(), push_mpls() and push_nsh(), which get
called from odp_execute_actions();
- In netdev_tnl_push_ip_header(), which gets called by the native
tunnels implementations, such as netdev_gre_push_header() for GRE, and is
ultimately called by push_tnl_action().
I haven't found a case where dp_packet_put_uninit() is actually used in a
DPBUF_DPDK packet. In all cases it seems to be a result of a packet created
locally by using dp_packet_new() or some variant.
What are your thoughts? Would be cool to hear other people's thoughts on this
as well.
Here's the diff of how this would look like. Mind you, this hasn't been run
whatsoever, only compiled.
Regards,
Tiago.
-=-=-=-=-=-=-=-=-=-=-=-
diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 443c225..ab01ca4 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -273,6 +273,21 @@ dp_packet_resize__(struct dp_packet *b, size_t
new_headroom, size_t new_tailroom
}
}
+static bool
+dp_packet_tailroom_avail(struct dp_packet *b, size_t size) { #ifdef
+DPDK_NETDEV
+if (b->source == DPBUF_DPDK) {
+if (size > dp_packet_tailroom(b)) {
+return false;
+}
+
+return true;
+}
+#endif
+return true;
+}
+
/* Ensures that 'b' has room for at least 'size' bytes at its tail end,
* reallocating and copying its data if necessary. Its headroom, if any, is
* preserved. */
@@ -284,6 +299,21 @@ dp_packet_prealloc_tailroom(struct dp_packet *b, size_t
size)
}
}
+static bool
+dp_packet_headroom_avail(struct dp_packet *b, size_t size) { #ifdef
+DPDK_NETDEV
+if (b->source == DPBUF_DPDK) {
+if (size > dp_packet_headroom(b)) {
+return false;
+}
+
+return true;
+}
+#endif
+return true;
+}
+
/* Ensures that 'b' has room for at least 'size' bytes at its head,
* reallocating and copying its data if necessary. Its tailroom, if any, is
* preserved. */
@@ -320,6 +350,11 @@ void *
dp_packet_put_uninit(struct dp_packet *b, size_t size) {
void *p;
+
+if (!dp_packet_tailroom_avail(b, size)) {
+return NULL;
+}
+
dp_packet_prealloc_tailroom(b, size);
p = dp_packet_tail(b);
dp_packet_set_size(b, dp_packet_size(b) + size); @@ -333,6 +368,9 @@ void
* dp_packet_put_zeros(struct dp_packet *b, size_t size) {
void *dst = dp_packet_put_uninit(b, size);
+if (!dst) {
+return NULL;
+}
memset(dst, 0, size);
ret
[ovs-dev] [PATCH v5] :Improved Packet Drop Statistics in OVS
Currently OVS maintains explicit packet drop/error counters only on port level. Packets that are dropped as part of normal OpenFlow processing are counted in flow stats of “drop” flows or as table misses in table stats. These can only be interpreted by controllers that know the semantics of the configured OpenFlow pipeline. Without that knowledge, it is impossible for an OVS user to obtain e.g. the total number of packets dropped due to OpenFlow rules. Furthermore, there are numerous other reasons for which packets can be dropped by OVS slow path that are not related to the OpenFlow pipeline. The generated datapath flow entries include a drop action to avoid further expensive upcalls to the slow path, but subsequent packets dropped by the datapath are not accounted anywhere. Finally, the datapath itself drops packets in certain error situations. Also, these drops are today not accounted for. This makes it difficult for OVS users to monitor packet drop in an OVS instance and to alert a management system in case of a unexpected increase of such drops. Also OVS trouble-shooters face difficulties in analysing packet drops. With this patch we implement following changes to address the issues mentioned above. 1. Account and categorize all the packet drops in OVS. 2. Account & classify “drop” action packet drops according to the drop reason. 3. Identify and account all the silent packet drop scenarios. 4. Display these drops in ovs-appctl coverage/show 5. Modified ovs-appctl dpcls/dump-flows and ovs-appctl dpif/dump-flows to print drop reason along with drop action A detailed presentation on this was presented at OvS conference 2017 and link for the corresponding presentation is available at: https://www.slideshare.net/LF_OpenvSwitch/lfovs17troubleshooting-the-data-plane-in-ovs-82280329 Sample ovs-appctl dpcls/dump-flows & ovs-appctl dpif/dump-flows displaying drop reason along with drop action. The idea is to use the coverage infrastructure to maintain the drops $ ovs-appctl dpctl/dump-flows netdev at ovs-netdev flow-dump from pmd on cpu core: 0 recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:12, bytes:1176, used:0.884s, actions:drop:recursion too deep $ ovs-appctl dpif/dump-flows br-int recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:25, bytes:2450, used:5.008s, actions:drop:recursion too deep recirc_id(0),in_port(5),packet_type(ns=0,id=0),eth_type(0x0806), packets:7, bytes:294, used:0.009s, actions:drop:recursion too deep In subsequent commits, we are planning to see if we can use this infrastructure to create a wrapper to clear and display counters as well. Co-authored-by: Rohith Basavaraja Co-authored-by: Keshav Gupta Signed-off-by: Anju Thomas Signed-off-by: Rohith Basavaraja Signed-off-by: Keshav Gupta --- datapath/linux/compat/include/linux/openvswitch.h | 51 ++ lib/coverage.c| 6 +- lib/coverage.h| 3 +- lib/dpif-netdev.c | 105 +++- lib/dpif.c| 10 +- lib/dpif.h| 3 + lib/netdev-dpdk.c | 4 + lib/odp-execute.c | 50 -- lib/odp-execute.h | 10 +- lib/odp-util.c| 54 +- ofproto/ofproto-dpif-ipfix.c | 1 + ofproto/ofproto-dpif-sflow.c | 1 + ofproto/ofproto-dpif-upcall.c | 3 +- ofproto/ofproto-dpif-xlate.c | 64 +++ ofproto/ofproto-dpif-xlate.h | 4 + ofproto/ofproto-dpif.c| 11 +- ofproto/ofproto-dpif.h| 8 +- tests/automake.mk | 3 +- tests/bundle.at | 2 +- tests/classifier.at | 10 +- tests/dpif-netdev.at | 17 +- tests/drop-stats.at | 197 ++ tests/odp.at | 6 +- tests/ofproto-dpif.at | 118 ++--- tests/ovs-ofctl.at| 2 +- tests/packet-type-aware.at| 17 +- tests/testsuite.at| 1 + tests/tunnel-push-pop.at | 26 ++- tests/tunnel.at | 21 ++- 29 files changed, 698 insertions(+), 110 deletions(-) create mode 100644 tests/drop-stats.at diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapa
Re: [ovs-dev] [PATCH v5] Improved Packet Drop Statistics in OVS
Hi Gowrishankar,
Thanks for your comments & suggestions
Replies inline ..
Regards & Thanks
Anju
-Original Message-
From: Gowrishankar Muthukrishnan [mailto:gmuth...@redhat.com]
Sent: Thursday, September 27, 2018 4:28 PM
To: Keshav Gupta
Cc: d...@openvswitch.org; Anju Thomas
Subject: Re: [ovs-dev] [PATCH v5] Improved Packet Drop Statistics in OVS
Hi Keshav,
Thank you for the patches. They certainly help us debugging packet drops more
meaningfully. Following your changes, some of the places where additional
reasons for drop could also be added as I hope helpful. Could you please check
below items as well.
__netdev_dpdk_vhost_send() AND netdev_dpdk_send__():
drop from netdev_dpdk_filter_packet_len(), when packet length exceeds wrt
max based on mtu (DPIF_DROP_TX_INVALID_PACKET_DROP ?)
drop from netdev_dpdk_qos_run(), when egress policer drops the packets
(DPIF_DROP_TX_QOS_DROP ?)
[Anju] These are accounted as part of the Interface and policer drops. The idea
of this feature was to account for the earlier unaccounted drops so
We haven’t actually looked into dividing these into further categories. But I
think this is something we can consider to do as an enhancement
bundle_send_learning_packets():
Seems like it is used when bond needs mac learning. However, there is no
differentiation wrt error/drop when xlate_send_packet() would fail down the
line.
Also any plan to cover drops in dummy dpif for testing aspects ?. Eg.
eth_from_flow() could return NULL packet.
[Anju] We had planned to account for drops in the dpdk datapath. Hence we have
not considered drops in bundle_run and others which happens in the main thread.
For similar reasone we have not planning for dummy dpif for now. Again I feel
this is a very good idea for our future enhancements.
I have one comment which is inline below.
On 09/15/2018 01:33 AM, Keshav Gupta wrote:
> Currently OVS maintains explicit packet drop/error counters only on
> port level. Packets that are dropped as part of normal OpenFlow
> processing are counted in flow stats of “drop” flows or as table misses in
> table stats.
> These can only be interpreted by controllers that know the semantics
> of the configured OpenFlow pipeline. Without that knowledge, it is
> impossible for an OVS user to obtain e.g. the total number of packets
> dropped due to OpenFlow rules.
>
> Furthermore, there are numerous other reasons for which packets can be
> dropped by OVS slow path that are not related to the OpenFlow pipeline.
> The generated datapath flow entries include a drop action to avoid
> further expensive upcalls to the slow path, but subsequent packets
> dropped by the datapath are not accounted anywhere.
>
> Finally, the datapath itself drops packets in certain error situations.
> Also, these drops are today not accounted for.
>
> This makes it difficult for OVS users to monitor packet drop in an OVS
> instance and to alert a management system in case of a unexpected
> increase of such drops. Also OVS trouble-shooters face difficulties in
> analysing packet drops.
>
> With this patch we implement following changes to address the issues
> mentioned above.
> +static void
> dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
>const struct nlattr *a, bool should_steal)
> OVS_NO_THREAD_SAFETY_ANALYSIS
> @@ -6449,6 +6520,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
> *packets_,
> struct dp_netdev *dp = pmd->dp;
> int type = nl_attr_type(a);
> struct tx_port *p;
> +uint32_t packet_count, packet_dropped;
> +struct dpif_drop_counter cntr;
>
> switch ((enum ovs_action_attr)type) {
> case OVS_ACTION_ATTR_OUTPUT:
> @@ -6490,6 +6563,11 @@ dp_execute_cb(void *aux_, struct dp_packet_batch
> *packets_,
> dp_packet_batch_add(&p->output_pkts, packet);
dp_packet_batch_add__ would drop packet if dp_packet_batch has reached
NETDEV_MAX_BURST.
Could we add another counter (and map its reason) for it.
DPIF_RX_PKT_BATCH_FULL ?
[Anju]. Thanks for this . I will include this as well
Thanks,
Gowrishankar
> }
> return;
> +} else {
> +cntr.type = DPIF_DROP_TYPE_DP;
> +cntr.counter.dp = DPIF_DP_INVALID_PORT_DROP;
> +pmd_perf_update_drop_counter(&pmd->perf_stats, &cntr,
> + packets_->count);
> }
> break;
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v1] Load balancing not happening in case of select group action for MPLS tagged packets
OVS does not do load balancing for select group buckets in case of mpls tagged
packets.
After an MPLS pop action, the expectation is to trigger recirculation .
This recirculation will ensure an RSS re-computation which will ensure load
balancing in case of select group bucket.
Due to a missing return statement before bucket selection, the bucket selection
in case of select group happens
before the recirculation and hence no load balancing is achieved
Signed-off-by: Anju Thomas
---
ofproto/ofproto-dpif-xlate.c | 1 +
tests/mpls-xlate.at | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e26f6c8..fd9cac6 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4463,6 +4463,7 @@ pick_select_group(struct xlate_ctx *ctx, struct
group_dpif *group)
*/
if (ctx->was_mpls) {
ctx_trigger_freeze(ctx);
+return NULL;
}
switch (group->selection_method) {
diff --git a/tests/mpls-xlate.at b/tests/mpls-xlate.at
index e335b7f..4392f7b 100644
--- a/tests/mpls-xlate.at
+++ b/tests/mpls-xlate.at
@@ -78,8 +78,8 @@ done
AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/packets.*actions:1/actions:1/'
| strip_used | strip_ufid | sort], [0], [dnl
flow-dump from non-dpdk interfaces:
-recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x8847),mpls(label=22/0xf,tc=0/0,ttl=64/0x0,bos=1/1),
packets:3, bytes:54, used:0.0s, actions:pop_mpls(eth_type=0x800),recirc(0x3)
-recirc_id(0x3),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
actions:100
+recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x8847),mpls(label=22/0xf,tc=0/0,ttl=64/0x0,bos=1/1),
packets:3, bytes:54, used:0.0s, actions:pop_mpls(eth_type=0x800),recirc(0x3)
+recirc_id(0x3),in_port(1),packet_type(ns=0,id=0),eth(src=f8:bc:12:44:34:b6,dst=f8:bc:12:46:58:e0),eth_type(0x0800),ipv4(src=0.0.0.0,dst=0.0.0.0,proto=0,frag=no),
actions:100
])
dnl Test MPLS pop then all group output (bucket actions do not trigger
recirculation)
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Ben, We are facing multiple such crashes on different computes in our deployments. Seems to be a pretty common problem in our setup. As you suggested, it would be good if we can make the below changes as well.How do you suggest we move forward regarding the approaches? Regards & thanks Anju -Original Message- From: Anju Thomas Sent: Tuesday, June 05, 2018 10:26 AM To: Ben Pfaff Cc: d...@openvswitch.org Subject: RE: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action Hi , Any suggestions ? Will it be ok if we merge the tunnel push change that we have discussed below while I can work on the other changes in parallel? Regards & thanks Anju -Original Message- From: Anju Thomas Sent: Thursday, May 17, 2018 3:52 PM To: 'Ben Pfaff' Cc: d...@openvswitch.org Subject: RE: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action Hi Ben, I was working on the code changes and I can think of two approaches we can take to prevent this crash in the dpdk datapath. 1. Today the dp_packet module that we have is never return any error . The only error handling it is by assert using OVS_NOT_REACHED(). Should we go ahead and restructure the entire dp_packet module to do some error handling and return (refer to openvswitchlib/dp-packet.c). This would require a more detailed effort to ensure all flows are handled. 2. Do not call the dp_packet_uninit without properly checking we have adequate headroom for DPBUF_DPDK buffers. Any suggestions on which approach we should go ahead with ? Regards & Thanks Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Friday, May 11, 2018 2:00 AM To: Anju Thomas Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action On Thu, May 10, 2018 at 08:51:03AM +, Anju Thomas wrote: > It looks like your commit message describes at least two other bugs in > OVS, though. First, if OVS crashes when it pushes tunnel headers, > even if there's not enough headroom, that's really bad. At worst, it > should drop the packet. Do you know where the crash occurs? We > should fix the problem, since it might recur in some other context. > > [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual > assert in our case. > The rootcause is that dp receives the actions after the upcall (say with >=3 > tunnel pushes ) . Now as part of action processing , since it is a tunnel > push action , we try to find space in the dpdk mbuf packet headroom (which Is > 128 bytes). By the time we try to process the third tunnel push , there is no > headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes > headroom). Hence it manually asserts . This assert is to catch any > unexpected code flow . Do you think that in this case we should still go > ahead and prevent the crash ? I don't understand why it's OK to crash in this case. Why do you think so? > Second, it's a little shocking to hear that an encapsulation action without a > following output action causes a memory leak. We also need to fix that. Do > you have any more details? > [Anju] Now as explained above, the crash happens because we run out of > headroom. But in case we have say 2 or less than 2 tunnel pushes we will have > a mem leak as packet is never freed because the tnl push is the dp last > action and there is no other output action or any other action like recirc > that may translate to output action in the end leading to packet buffer not > being freed. > Are you proposing that we have some sort of preventive fix in the dp to > handle an incorrect action list from the upcall handling? Yes. It's unacceptable to leak memory because there's a packet modification without an output action. The kernel datapath never does this, for example. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi , Any suggestions ? Will it be ok if we merge the tunnel push change that we have discussed below while I can work on the other changes in parallel? Regards & thanks Anju -Original Message- From: Anju Thomas Sent: Thursday, May 17, 2018 3:52 PM To: 'Ben Pfaff' Cc: d...@openvswitch.org Subject: RE: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action Hi Ben, I was working on the code changes and I can think of two approaches we can take to prevent this crash in the dpdk datapath. 1. Today the dp_packet module that we have is never return any error . The only error handling it is by assert using OVS_NOT_REACHED(). Should we go ahead and restructure the entire dp_packet module to do some error handling and return (refer to openvswitchlib/dp-packet.c). This would require a more detailed effort to ensure all flows are handled. 2. Do not call the dp_packet_uninit without properly checking we have adequate headroom for DPBUF_DPDK buffers. Any suggestions on which approach we should go ahead with ? Regards & Thanks Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Friday, May 11, 2018 2:00 AM To: Anju Thomas Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action On Thu, May 10, 2018 at 08:51:03AM +, Anju Thomas wrote: > It looks like your commit message describes at least two other bugs in > OVS, though. First, if OVS crashes when it pushes tunnel headers, > even if there's not enough headroom, that's really bad. At worst, it > should drop the packet. Do you know where the crash occurs? We > should fix the problem, since it might recur in some other context. > > [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual > assert in our case. > The rootcause is that dp receives the actions after the upcall (say with >=3 > tunnel pushes ) . Now as part of action processing , since it is a tunnel > push action , we try to find space in the dpdk mbuf packet headroom (which Is > 128 bytes). By the time we try to process the third tunnel push , there is no > headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes > headroom). Hence it manually asserts . This assert is to catch any > unexpected code flow . Do you think that in this case we should still go > ahead and prevent the crash ? I don't understand why it's OK to crash in this case. Why do you think so? > Second, it's a little shocking to hear that an encapsulation action without a > following output action causes a memory leak. We also need to fix that. Do > you have any more details? > [Anju] Now as explained above, the crash happens because we run out of > headroom. But in case we have say 2 or less than 2 tunnel pushes we will have > a mem leak as packet is never freed because the tnl push is the dp last > action and there is no other output action or any other action like recirc > that may translate to output action in the end leading to packet buffer not > being freed. > Are you proposing that we have some sort of preventive fix in the dp to > handle an incorrect action list from the upcall handling? Yes. It's unacceptable to leak memory because there's a packet modification without an output action. The kernel datapath never does this, for example. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Ben, I was working on the code changes and I can think of two approaches we can take to prevent this crash in the dpdk datapath. 1. Today the dp_packet module that we have is never return any error . The only error handling it is by assert using OVS_NOT_REACHED(). Should we go ahead and restructure the entire dp_packet module to do some error handling and return (refer to openvswitchlib/dp-packet.c). This would require a more detailed effort to ensure all flows are handled. 2. Do not call the dp_packet_uninit without properly checking we have adequate headroom for DPBUF_DPDK buffers. Any suggestions on which approach we should go ahead with ? Regards & Thanks Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Friday, May 11, 2018 2:00 AM To: Anju Thomas Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action On Thu, May 10, 2018 at 08:51:03AM +0000, Anju Thomas wrote: > It looks like your commit message describes at least two other bugs in > OVS, though. First, if OVS crashes when it pushes tunnel headers, > even if there's not enough headroom, that's really bad. At worst, it > should drop the packet. Do you know where the crash occurs? We > should fix the problem, since it might recur in some other context. > > [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual > assert in our case. > The rootcause is that dp receives the actions after the upcall (say with >=3 > tunnel pushes ) . Now as part of action processing , since it is a tunnel > push action , we try to find space in the dpdk mbuf packet headroom (which Is > 128 bytes). By the time we try to process the third tunnel push , there is no > headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes > headroom). Hence it manually asserts . This assert is to catch any > unexpected code flow . Do you think that in this case we should still go > ahead and prevent the crash ? I don't understand why it's OK to crash in this case. Why do you think so? > Second, it's a little shocking to hear that an encapsulation action without a > following output action causes a memory leak. We also need to fix that. Do > you have any more details? > [Anju] Now as explained above, the crash happens because we run out of > headroom. But in case we have say 2 or less than 2 tunnel pushes we will have > a mem leak as packet is never freed because the tnl push is the dp last > action and there is no other output action or any other action like recirc > that may translate to output action in the end leading to packet buffer not > being freed. > Are you proposing that we have some sort of preventive fix in the dp to > handle an incorrect action list from the upcall handling? Yes. It's unacceptable to leak memory because there's a packet modification without an output action. The kernel datapath never does this, for example. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Ilya,
I had a look at the patch that you have submitted.
In push_tnl_action, I can see that with the patch you are trying to return
error in case we don’t find the tunnel port or return the error that
netdev_push header returns. And we are deciding to delete the batch in the
calling function dp_execute_cb in case of error. But netdev_push_header always
returns 0. Actually the following code in push_tnl_action is dead code.
if (!err) {
return 0;
}
The actual issue is that the push_header function always return void . So any
error happening during push_header is never captured or acted upon.
I think the fix also be should be to add/capture error scenarios during tnl
header push as well.
Regards & Thanks
Anju
-Original Message-
From: Ilya Maximets [mailto:i.maxim...@samsung.com]
Sent: Monday, May 14, 2018 6:28 PM
To: ovs-dev@openvswitch.org; Anju Thomas ; Ben Pfaff
Cc: Tiago Lam
Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hello Anju, Ben,
Looks like I fixed a leak reported here in my recent patch:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346892.html
Could you, please, take a look at it?
I actually managed to reproduce the packet leak on tunnel-push-pop unit tests
with valgrind:
==6445== 2,912 (2,208 direct, 704 indirect) bytes in 4 blocks are definitely
lost in loss record 400 of 410
==6445==at 0x4C2DB8F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6445==by 0x516314: xmalloc (util.c:120)
==6445==by 0x466154: dp_packet_new (dp-packet.c:138)
==6445==by 0x466154: dp_packet_new_with_headroom (dp-packet.c:148)
==6445==by 0x4F6CFE: eth_from_hex (packets.c:498)
==6445==by 0x48EB43: eth_from_packet (netdev-dummy.c:1450)
==6445==by 0x48EB43: netdev_dummy_receive (netdev-dummy.c:1562)
==6445==by 0x515980: process_command (unixctl.c:313)
==6445==by 0x515980: run_connection (unixctl.c:347)
==6445==by 0x515980: unixctl_server_run (unixctl.c:398)
==6445==by 0x406F1E: main (ovs-vswitchd.c:120)
Above patch fixes it.
Best regards, Ilya Maximets.
> Hi Ben,
>
> I will work on these changes as well.
>
> Regards
> Anju
>
> -Original Message-
> From: Ben Pfaff [mailto:blp at ovn.org]
> Sent: Friday, May 11, 2018 2:00 AM
> To: Anju Thomas
> Cc: dev at openvswitch.org
> Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push
> action
>
> On Thu, May 10, 2018 at 08:51:03AM +, Anju Thomas wrote:
>> It looks like your commit message describes at least two other bugs
>> in OVS, though. First, if OVS crashes when it pushes tunnel headers,
>> even if there's not enough headroom, that's really bad. At worst, it
>> should drop the packet. Do you know where the crash occurs? We
>> should fix the problem, since it might recur in some other context.
>>
>> [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual
>> assert in our case.
>> The rootcause is that dp receives the actions after the upcall (say with >=3
>> tunnel pushes ) . Now as part of action processing , since it is a tunnel
>> push action , we try to find space in the dpdk mbuf packet headroom (which
>> Is 128 bytes). By the time we try to process the third tunnel push , there
>> is no headroom left since each tunnel header is of 50 bytes (50 *3 > 128
>> bytes headroom). Hence it manually asserts . This assert is to catch any
>> unexpected code flow . Do you think that in this case we should still go
>> ahead and prevent the crash ?
>
> I don't understand why it's OK to crash in this case. Why do you think so?
>
>> Second, it's a little shocking to hear that an encapsulation action without
>> a following output action causes a memory leak. We also need to fix that.
>> Do you have any more details?
>> [Anju] Now as explained above, the crash happens because we run out of
>> headroom. But in case we have say 2 or less than 2 tunnel pushes we will
>> have a mem leak as packet is never freed because the tnl push is the dp last
>> action and there is no other output action or any other action like recirc
>> that may translate to output action in the end leading to packet buffer not
>> being freed.
>> Are you proposing that we have some sort of preventive fix in the dp to
>> handle an incorrect action list from the upcall handling?
>
> Yes. It's unacceptable to leak memory because there's a packet modification
> without an output action. The kernel datapath never does this, for example.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Ben, I will work on these changes as well. Regards Anju -Original Message- From: Ben Pfaff [mailto:b...@ovn.org] Sent: Friday, May 11, 2018 2:00 AM To: Anju Thomas Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action On Thu, May 10, 2018 at 08:51:03AM +, Anju Thomas wrote: > It looks like your commit message describes at least two other bugs in > OVS, though. First, if OVS crashes when it pushes tunnel headers, > even if there's not enough headroom, that's really bad. At worst, it > should drop the packet. Do you know where the crash occurs? We > should fix the problem, since it might recur in some other context. > > [Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual > assert in our case. > The rootcause is that dp receives the actions after the upcall (say with >=3 > tunnel pushes ) . Now as part of action processing , since it is a tunnel > push action , we try to find space in the dpdk mbuf packet headroom (which Is > 128 bytes). By the time we try to process the third tunnel push , there is no > headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes > headroom). Hence it manually asserts . This assert is to catch any > unexpected code flow . Do you think that in this case we should still go > ahead and prevent the crash ? I don't understand why it's OK to crash in this case. Why do you think so? > Second, it's a little shocking to hear that an encapsulation action without a > following output action causes a memory leak. We also need to fix that. Do > you have any more details? > [Anju] Now as explained above, the crash happens because we run out of > headroom. But in case we have say 2 or less than 2 tunnel pushes we will have > a mem leak as packet is never freed because the tnl push is the dp last > action and there is no other output action or any other action like recirc > that may translate to output action in the end leading to packet buffer not > being freed. > Are you proposing that we have some sort of preventive fix in the dp to > handle an incorrect action list from the upcall handling? Yes. It's unacceptable to leak memory because there's a packet modification without an output action. The kernel datapath never does this, for example. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v1] Fix crash when processing malformed Bundle Add message in OVS
Hi Ben,
Yes. This patch certainly looks more cleaner and better to me .
Thanks
Anju
-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org]
Sent: Thursday, May 10, 2018 2:33 AM
To: Anju Thomas
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1] Fix crash when processing malformed Bundle
Add message in OVS
On Mon, May 07, 2018 at 10:58:06PM +0530, Anju Thomas wrote:
> When an OpenFlow Bundle Add message is received, a bundle entry is
> created and the OpenFlow message embedded in the bundle add message is
> processed. If any error is encountered while processing the embedded
> message, the bundle entry is freed. The bundle entry free function
> assumes that the entry has been populated with a properly formatted
> OpenFlow message and performs some message specific cleanup actions .
> This assumption does not hold true in the error case and OVS crashes
> when performing the cleanup.
>
> The fix is in case of errors, simply free the bundle entry without
> attempting to perform any embedded message cleanup
>
> Signed-off-by: Anju Thomas
Thanks for the fix.
It's not really nice to have multiple levels of initialized-ness. What do you
think of this fix instead?
Thanks,
Ben.
--8<--cut here-->8--
From: Anju Thomas
Date: Mon, 7 May 2018 22:58:06 +0530
Subject: [PATCH] Fix crash when processing malformed Bundle Add message in OVS
When an OpenFlow Bundle Add message is received, a bundle entry is created and
the OpenFlow message embedded in the bundle add message is processed. If any
error is encountered while processing the embedded message, the bundle entry is
freed. The bundle entry free function assumes that the entry has been populated
with a properly formatted OpenFlow message and performs some message specific
cleanup actions .
This assumption does not hold true in the error case and OVS crashes when
performing the cleanup.
The fix is in case of errors, simply free the bundle entry without attempting
to perform any embedded message cleanup
Signed-off-by: Anju Thomas
Signed-off-by: Ben Pfaff
---
ofproto/bundles.h | 13 -
ofproto/ofproto.c | 21 +
2 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/ofproto/bundles.h b/ofproto/bundles.h index
806e853d6883..1164fddd8702 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -58,8 +58,6 @@ struct ofp_bundle {
struct ovs_list msg_list; /* List of 'struct bundle_message's */
};
-static inline struct ofp_bundle_entry *ofp_bundle_entry_alloc(
-enum ofptype type, const struct ofp_header *oh);
static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *);
enum ofperr ofp_bundle_open(struct ofconn *, uint32_t id, uint16_t flags, @@
-72,17 +70,6 @@ enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t id,
void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *);
-static inline struct ofp_bundle_entry * -ofp_bundle_entry_alloc(enum ofptype
type, const struct ofp_header *oh) -{
-struct ofp_bundle_entry *entry = xmalloc(sizeof *entry);
-
-entry->type = type;
-entry->msg = xmemdup(oh, ntohs(oh->length));
-
-return entry;
-}
-
static inline void
ofp_bundle_entry_free(struct ofp_bundle_entry *entry) { diff --git
a/ofproto/ofproto.c b/ofproto/ofproto.c index 36f4c0b16d48..565a45db1507 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -7899,7 +7899,6 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
enum ofperr error;
struct ofputil_bundle_add_msg badd;
-struct ofp_bundle_entry *bmsg;
enum ofptype type;
error = reject_slave_controller(ofconn); @@ -7912,7 +7911,8 @@
handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
return error;
}
-bmsg = ofp_bundle_entry_alloc(type, badd.msg);
+/* Allocate bundle entry and decode the embedded message. */
+struct ofp_bundle_entry *bmsg = xmalloc(sizeof *bmsg);
struct ofpbuf ofpacts;
uint64_t ofpacts_stub[1024 / 8];
@@ -7951,18 +7951,23 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
} else {
OVS_NOT_REACHED();
}
-
ofpbuf_uninit(&ofpacts);
-
-if (!error) {
-error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
- bmsg, oh);
+if (error) {
+free(bmsg);
+return error;
}
+/* Now that the embedded message has been successfully decoded, finish up
+ * initializing the bundle entry. */
+bmsg->type = type;
+bmsg->msg = xmemdup(oh, ntohs(oh->length));
+
+/* Add bundle entry to bundle. */
+error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags,
+ bmsg, oh);
if (error)
Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
Hi Ben,
Replies inline:
Regards & thanks
Anju
-Original Message-
From: Ben Pfaff [mailto:b...@ovn.org]
Sent: Thursday, May 10, 2018 1:59 AM
To: Anju Thomas
Cc: d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
On Tue, May 08, 2018 at 12:34:54AM +0530, Anju Thomas wrote:
> During slow path packet processing, if the action is to output to a
> tunnel port, the slow path processing of the encapsulated packet
> continues on the underlay bridge and additional actions (e.g. optional
> VLAN encapsulation, bond link selection and finally output to port)
> are collected there.
>
> To prepare for a continuation of the processing of the original packet
> (e.g. output to other tunnel ports in a flooding scenario), the
> “tunnel_push” action and the actions of the underlay bridge are
> encapsulated in a clone() action to preserve the original packet.
>
> If the underlay bridge decides to drop the tunnel packet (for example
> if both bonded ports are down simultaneously), the clone(tunnel_push))
> actions previously generated as part of translation of the output to
> tunnel port are discarded and a stand-alone tunnel_push action is
> added instead. Thus the tunnel header is pushed on to the original packet.
> This is the bug.
>
> Consequences: If packet processing continues with sending to further
> tunnel ports, multiple tunnel header pushes will happen on the
> original packet as typically the tunnels all traverse the same
> underlay bond which is down. The packet may not have enough headroom
> to accommodate all the tunnel headers. OVS crashes if it runs out of
> space while trying to push the tunnel headers.
>
> Even in case there is enough headroom, the packet will not be freed
> since the accumulated action list contains only the tunnel header push
> action without any output port action. Thus, we either have a crash or
> a packet buffer leak.
>
> Signed-off-by: Anju Thomas
Thanks for the patch. It applies OK and all the tests pass.
It looks like your commit message describes at least two other bugs in OVS,
though. First, if OVS crashes when it pushes tunnel headers, even if there's
not enough headroom, that's really bad. At worst, it should drop the packet.
Do you know where the crash occurs? We should fix the problem, since it might
recur in some other context.
[Anju] OVS was actually crashing in the dpdk datapath. The crash is a manual
assert in our case.
The rootcause is that dp receives the actions after the upcall (say with >=3
tunnel pushes ) . Now as part of action processing , since it is a tunnel push
action , we try to find space in the dpdk mbuf packet headroom (which Is 128
bytes). By the time we try to process the third tunnel push , there is no
headroom left since each tunnel header is of 50 bytes (50 *3 > 128 bytes
headroom). Hence it manually asserts . This assert is to catch any unexpected
code flow . Do you think that in this case we should still go ahead and
prevent the crash ?
The back trace was as below:-
(gdb) bt
#0 0x7ffa9a856c37 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1 0x7ffa9a85a028 in __GI_abort () at abort.c:89
#2 0x005fc725 in dp_packet_resize__ (b=b@entry=0x7ffa87744680,
new_headroom=new_headroom@entry=64, new_tailroom=)
at lib/dp-packet.c:258
#3 0x005fcb1f in dp_packet_prealloc_headroom
(b=b@entry=0x7ffa87744680, size=size@entry=50) at lib/dp-packet.c:288
#4 0x005fcf91 in dp_packet_push_uninit (b=b@entry=0x7ffa87744680,
size=size@entry=50) at lib/dp-packet.c:400
#5 0x0069466c in netdev_tnl_push_ip_header
(packet=packet@entry=0x7ffa87744680, header=0x7ff85401bab0, size=50,
ip_tot_size=ip_tot_size@entry=0x7ff8117f89fc) at lib/netdev-native-tnl.c:152
#6 0x0069472a in netdev_tnl_push_udp_header (packet=0x7ffa87744680,
data=) at lib/netdev-native-tnl.c:215
#7 0x00625627 in netdev_push_header (netdev=0x3ca3990,
batch=batch@entry=0x7ff8117f9498, data=data@entry=0x7ff85401baa0)
at lib/netdev.c:874
#8 0x006069f2 in push_tnl_action (batch=0x7ff8117f9498,
attr=0x7ff85401ba9c, pmd=0x33efb30) at lib/dpif-netdev.c:4629
#9 dp_execute_cb (aux_=aux_@entry=0x7ff8117f9840,
packets_=packets_@entry=0x7ff8117f9498, a=a@entry=0x7ff85401ba9c,
may_steal=false)
static void
dp_packet_resize__(struct dp_packet *b, size_t new_headroom, size_t
new_tailroom)
{
void *new_base, *new_data;
size_t new_allocated;
new_allocated = new_headroom + dp_packet_size(b) + new_tailroom;
switch (b->source) {
case DPBUF_DPDK:
OVS_NOT_REACHED();-->crashes
here
Second, it's a little shocking to hear that an encapsulation action witho
[ovs-dev] [PATCH v1] Fix crash due to multiple tnl push action
During slow path packet processing, if the action is to output to a
tunnel port, the slow path processing of the encapsulated packet
continues on the underlay bridge and additional actions (e.g. optional
VLAN encapsulation, bond link selection and finally output to port) are
collected there.
To prepare for a continuation of the processing of the original packet
(e.g. output to other tunnel ports in a flooding scenario), the
“tunnel_push” action and the actions of the underlay bridge are
encapsulated in a clone() action to preserve the original packet.
If the underlay bridge decides to drop the tunnel packet (for example if
both bonded ports are down simultaneously), the clone(tunnel_push))
actions previously generated as part of translation of the output to
tunnel port are discarded and a stand-alone tunnel_push action is added
instead. Thus the tunnel header is pushed on to the original packet.
This is the bug.
Consequences: If packet processing continues with sending to further
tunnel ports, multiple tunnel header pushes will happen on the original
packet as typically the tunnels all traverse the same underlay bond
which is down. The packet may not have enough headroom to accommodate
all the tunnel headers. OVS crashes if it runs out of space while trying
to push the tunnel headers.
Even in case there is enough headroom, the packet will not be freed
since the accumulated action list contains only the tunnel header push
action without any output port action. Thus, we either have a crash or a
packet buffer leak.
Signed-off-by: Anju Thomas
---
ofproto/ofproto-dpif-xlate.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 5641724..5706675 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3521,13 +3521,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct
xport *xport,
nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
} else {
nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
-/* XXX : There is no real use-case for a tunnel push without
- * any post actions. However keeping it now
- * as is to make the 'make check' happy. Should remove when all the
- * make check tunnel test case does something meaningful on a
- * tunnel encap packets.
- */
-odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
}
/* Restore context status. */
--
1.9.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v1] Fix crash when processing malformed Bundle Add message in OVS
When an OpenFlow Bundle Add message is received, a bundle entry is
created and the OpenFlow message embedded in the bundle add message is
processed. If any error is encountered while processing the embedded
message, the bundle entry is freed. The bundle entry free function
assumes that the entry has been populated with a properly formatted
OpenFlow message and performs some message specific cleanup actions .
This assumption does not hold true in the error case and OVS crashes
when performing the cleanup.
The fix is in case of errors, simply free the bundle entry without
attempting to perform any embedded message cleanup
Signed-off-by: Anju Thomas
---
ofproto/bundles.c | 2 +-
ofproto/bundles.h | 18 ++
ofproto/ofproto.c | 19 ---
3 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/ofproto/bundles.c b/ofproto/bundles.c
index 405ec8c..628ba0a 100644
--- a/ofproto/bundles.c
+++ b/ofproto/bundles.c
@@ -63,7 +63,7 @@ ofp_bundle_remove__(struct ofconn *ofconn, struct ofp_bundle
*bundle)
struct ofp_bundle_entry *msg;
LIST_FOR_EACH_POP (msg, node, &bundle->msg_list) {
-ofp_bundle_entry_free(msg);
+ofp_bundle_entry_free(msg, true);
}
ofconn_remove_bundle(ofconn, bundle);
diff --git a/ofproto/bundles.h b/ofproto/bundles.h
index 806e853..7fc731c 100644
--- a/ofproto/bundles.h
+++ b/ofproto/bundles.h
@@ -60,7 +60,7 @@ struct ofp_bundle {
static inline struct ofp_bundle_entry *ofp_bundle_entry_alloc(
enum ofptype type, const struct ofp_header *oh);
-static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *);
+static inline void ofp_bundle_entry_free(struct ofp_bundle_entry *, bool);
enum ofperr ofp_bundle_open(struct ofconn *, uint32_t id, uint16_t flags,
const struct ofp_header *);
@@ -84,15 +84,17 @@ ofp_bundle_entry_alloc(enum ofptype type, const struct
ofp_header *oh)
}
static inline void
-ofp_bundle_entry_free(struct ofp_bundle_entry *entry)
+ofp_bundle_entry_free(struct ofp_bundle_entry *entry, bool cleanup)
{
if (entry) {
-if (entry->type == OFPTYPE_FLOW_MOD) {
-ofproto_flow_mod_uninit(&entry->ofm);
-} else if (entry->type == OFPTYPE_GROUP_MOD) {
-ofputil_uninit_group_mod(&entry->ogm.gm);
-} else if (entry->type == OFPTYPE_PACKET_OUT) {
-ofproto_packet_out_uninit(&entry->opo);
+if (cleanup) {
+if (entry->type == OFPTYPE_FLOW_MOD) {
+ofproto_flow_mod_uninit(&entry->ofm);
+} else if (entry->type == OFPTYPE_GROUP_MOD) {
+ofputil_uninit_group_mod(&entry->ogm.gm);
+} else if (entry->type == OFPTYPE_PACKET_OUT) {
+ofproto_packet_out_uninit(&entry->opo);
+}
}
free(entry->msg);
free(entry);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 36f4c0b..bfbc043 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -7901,6 +7901,7 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
struct ofputil_bundle_add_msg badd;
struct ofp_bundle_entry *bmsg;
enum ofptype type;
+bool cleanup=true;
error = reject_slave_controller(ofconn);
if (error) {
@@ -7929,7 +7930,13 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
&ofproto->vl_mff_map, &ofpacts,
u16_to_ofp(ofproto->max_ports),
ofproto->n_tables);
-if (!error) {
+if (error) {
+VLOG_WARN("Unable to parse OFPTYPE_FLOW_MOD message in "
+ "OFPTYPE_BUNDLE_ADD_MESSAGE(bundle idx = %x)"
+ "successfully",
+ badd.bundle_id);
+cleanup=false;
+} else {
error = ofproto_flow_mod_init(ofproto, &bmsg->ofm, &fm, NULL);
minimatch_destroy(&fm.match);
}
@@ -7944,7 +7951,13 @@ handle_bundle_add(struct ofconn *ofconn, const struct
ofp_header *oh)
error = ofputil_decode_packet_out(&po, badd.msg,
ofproto_get_tun_tab(ofproto),
&ofpacts);
-if (!error) {
+if (error) {
+ VLOG_WARN("Unable to parse OFPTYPE_PACKET_OUT message in "
+"OFPTYPE_BUNDLE_ADD_MESSAGE(bundle idx = %x)"
+"successfully",
+ badd.bundle_id);
+ cleanup=false;
+} else {
po.ofpacts = ofpbuf_steal_data(&ofpacts); /* Move to heap. */
error = ofproto_packet_out_init(ofproto, ofconn, &bmsg->opo, &po);
}
@@ -7960,7 +7973,
