Re: [ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
On 4/27/2021 6:57 PM, Sriharsha Basavapatna wrote: On Tue, Apr 27, 2021 at 6:42 PM Eli Britstein wrote: On 4/27/2021 2:45 PM, Sriharsha Basavapatna wrote: On Tue, Apr 27, 2021 at 4:26 PM Ilya Maximets wrote: On 4/27/21 11:56 AM, Sriharsha Basavapatna via dev wrote: Hi Eli, On Sun, Apr 25, 2021 at 6:22 PM Eli Britstein wrote: Hi Harsha, On 4/20/2021 11:07 AM, Sriharsha Basavapatna wrote: Sometimes a port might be configured with a single flow that just forwards packets to another port. This would be useful in configs where the bridge is just fowarding packets between two ports (for example, between a vhost-user port and a physical port). A flow that matches only on the in_port and with an action that forwards to another port would be configured, to avoid learning or matching on packet headers. Example: $ ovs-ofctl add-flow br0 in_port=1,actions=output:2 $ ovs-ofctl add-flow br0 in_port=2,actions=output:1 This translates to a datapath flow with the match fields wildcarded for the packet headers. However, the datapath processing still involves There are still several matches (not wildcards): - recirc_id - in_port - packet_type - dl_type - vlan_tci - nw_frag (for ip packets) So there might be multiple flows for each such openflow rule. In the past, I have tried to optimize such scenario, see: https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/357882.html That was wrong as commented afterwards. Another related patch-set was this (also not accepted): https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363948.html Ilya wrote an alternative patch: https://patchwork.ozlabs.org/patch/1105880/ AFAIR, it didn't improve performance either. Would be good to have some performance numbers for it as there was no test results published and I don't know if someone ever tested it. Thanks for the above pointers. Ilya had also shared this patch recently while discussing this topic at the ovs-dpdk community meeting. I want to see if we can utilize part of the logic in that patch to add some constraints, while still avoiding an additional table/lookup. The 'port-forward' mode implies that the user wants to avoid any kind of lookup in the datapath (as indicated by the ofctl rule + port-forward mode). I don't see how to completely avoid lookups. IIUC, in this patch there is a match and upcall for the first packet, but there are no matches for subsequent packets. That's right. Allow the first packet to go through match, upcall, dp/cache insertion etc. For subsequent packets avoid lookup. This will work only for flow actions that doesn't modify the packet. If for some reason the flow contains header modifications OVS will not do that correctly because the header is not parsed. Also, if the packet is a bit different from the very first packet, we might attempt to modify headers that doesn't exist. All in all, this is very dangerous and might lead to OVS crash. We can't rely on the user to set specific OF rules for this functionality and we should not have a feature that might crash OVS if not used accurately. The way to not parse the packet at all and to not perform any matches is the way to completely ignore OF rules, but OVS is an OF switch and such functionality just doesn't fit. If I add a constraint to check that there is only one action and it's an OUTPUT action (i.e don't enable port-forward mode if the DP flow contains other actions like modify), like it is done in your patch, that should handle this issue ? Thanks, -Harsha In my change I minimized the lookup as possible to a single 64bit key. And it will actually work with any OF rules and without enabling of any special flags. Would be great to see some performance numbers for it as I didn't see any. With pvp tests (vxlan config), we have seen better performance both in pps: ~50% and cpp: ~35%, at a few thousand flows. Similar improvement can be seen with simple configurations (e.g testpmd in the vm in txonly fwd mode). Besides, I've tried this patch. Maybe I did something wrong (I configured port-forward=true on those ports and those openflow rules, and pinged between those ports). I didn't see it worked (the coverage, and also I added my own prints). When you enable port-forward and start the traffic, you should see a message like this: "dpif_netdev(pmd-c02/id:74)|DBG|Setting port_forward_flow: port: 0x7f63400050b0 flow: 0x7f634000afb0" I'm guessing the flow isn't getting added to the port; the insertion is currently done when there's an emc hit. I should probably move the insertion code to handle_packet_upcall(). As a workaround, can you please update the emc insertion probability (ovs-vsctl --no-wait set Open_vSwitch . other_config:emc-insert-inv-prob=1) and retry your test ? Also, please disable normal mode in the bridge (ovs-ofctl del-flows br0; and then add ofctl rules). Let me know if you still see the problem, I'll work with you offline. With this proposed patch,
Re: [ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
On Tue, Apr 27, 2021 at 6:42 PM Eli Britstein wrote: > > > On 4/27/2021 2:45 PM, Sriharsha Basavapatna wrote: > > On Tue, Apr 27, 2021 at 4:26 PM Ilya Maximets wrote: > >> On 4/27/21 11:56 AM, Sriharsha Basavapatna via dev wrote: > >>> Hi Eli, > >>> > >>> On Sun, Apr 25, 2021 at 6:22 PM Eli Britstein wrote: > Hi Harsha, > > On 4/20/2021 11:07 AM, Sriharsha Basavapatna wrote: > > Sometimes a port might be configured with a single flow that just > > forwards packets to another port. This would be useful in configs > > where the bridge is just fowarding packets between two ports (for > > example, between a vhost-user port and a physical port). A flow > > that matches only on the in_port and with an action that forwards > > to another port would be configured, to avoid learning or matching > > on packet headers. > > > > Example: > > $ ovs-ofctl add-flow br0 in_port=1,actions=output:2 > > $ ovs-ofctl add-flow br0 in_port=2,actions=output:1 > > > > This translates to a datapath flow with the match fields wildcarded > > for the packet headers. However, the datapath processing still involves > There are still several matches (not wildcards): > > - recirc_id > - in_port > - packet_type > - dl_type > - vlan_tci > - nw_frag (for ip packets) > > So there might be multiple flows for each such openflow rule. > > In the past, I have tried to optimize such scenario, see: > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/357882.html > > That was wrong as commented afterwards. > > Another related patch-set was this (also not accepted): > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363948.html > > Ilya wrote an alternative patch: > > https://patchwork.ozlabs.org/patch/1105880/ > > AFAIR, it didn't improve performance either. > >> Would be good to have some performance numbers for it as there was > >> no test results published and I don't know if someone ever tested it. > >> > >>> Thanks for the above pointers. Ilya had also shared this patch > >>> recently while discussing this topic at the ovs-dpdk community > >>> meeting. I want to see if we can utilize part of the logic in that > >>> patch to add some constraints, while still avoiding an additional > >>> table/lookup. The 'port-forward' mode implies that the user wants to > >>> avoid any kind of lookup in the datapath (as indicated by the ofctl > >>> rule + port-forward mode). > >> I don't see how to completely avoid lookups. > >> > >> IIUC, in this patch there is a match and upcall for the first packet, > >> but there are no matches for subsequent packets. > > That's right. Allow the first packet to go through match, upcall, > > dp/cache insertion etc. For subsequent packets avoid lookup. > > > >> This will work > >> only for flow actions that doesn't modify the packet. If for some > >> reason the flow contains header modifications OVS will not do that > >> correctly because the header is not parsed. Also, if the packet is > >> a bit different from the very first packet, we might attempt to > >> modify headers that doesn't exist. All in all, this is very dangerous > >> and might lead to OVS crash. We can't rely on the user to set specific > >> OF rules for this functionality and we should not have a feature that > >> might crash OVS if not used accurately. > >> > >> The way to not parse the packet at all and to not perform any matches is > >> the way to completely ignore OF rules, but OVS is an OF switch and > >> such functionality just doesn't fit. > > If I add a constraint to check that there is only one action and it's > > an OUTPUT action (i.e don't enable port-forward mode if the DP flow > > contains other actions like modify), like it is done in your patch, > > that should handle this issue ? > > > > Thanks, > > -Harsha > >> In my change I minimized the lookup as possible to a single 64bit key. > >> And it will actually work with any OF rules and without enabling of > >> any special flags. Would be great to see some performance numbers > >> for it as I didn't see any. > >> > >>> With pvp tests (vxlan config), we have > >>> seen better performance both in pps: ~50% and cpp: ~35%, at a few > >>> thousand flows. Similar improvement can be seen with simple > >>> configurations (e.g testpmd in the vm in txonly fwd mode). > >>> > Besides, I've tried this patch. Maybe I did something wrong (I > configured port-forward=true on those ports and those openflow rules, > and pinged between those ports). I didn't see it worked (the coverage, > and also I added my own prints). > >>> When you enable port-forward and start the traffic, you should see a > >>> message like this: > >>> "dpif_netdev(pmd-c02/id:74)|DBG|Setting port_forward_flow: port: > >>> 0x7f63400050b0 flow: 0x7f634000afb0" > >>> > >>>
Re: [ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
On 4/27/2021 2:45 PM, Sriharsha Basavapatna wrote: On Tue, Apr 27, 2021 at 4:26 PM Ilya Maximets wrote: On 4/27/21 11:56 AM, Sriharsha Basavapatna via dev wrote: Hi Eli, On Sun, Apr 25, 2021 at 6:22 PM Eli Britstein wrote: Hi Harsha, On 4/20/2021 11:07 AM, Sriharsha Basavapatna wrote: Sometimes a port might be configured with a single flow that just forwards packets to another port. This would be useful in configs where the bridge is just fowarding packets between two ports (for example, between a vhost-user port and a physical port). A flow that matches only on the in_port and with an action that forwards to another port would be configured, to avoid learning or matching on packet headers. Example: $ ovs-ofctl add-flow br0 in_port=1,actions=output:2 $ ovs-ofctl add-flow br0 in_port=2,actions=output:1 This translates to a datapath flow with the match fields wildcarded for the packet headers. However, the datapath processing still involves There are still several matches (not wildcards): - recirc_id - in_port - packet_type - dl_type - vlan_tci - nw_frag (for ip packets) So there might be multiple flows for each such openflow rule. In the past, I have tried to optimize such scenario, see: https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/357882.html That was wrong as commented afterwards. Another related patch-set was this (also not accepted): https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363948.html Ilya wrote an alternative patch: https://patchwork.ozlabs.org/patch/1105880/ AFAIR, it didn't improve performance either. Would be good to have some performance numbers for it as there was no test results published and I don't know if someone ever tested it. Thanks for the above pointers. Ilya had also shared this patch recently while discussing this topic at the ovs-dpdk community meeting. I want to see if we can utilize part of the logic in that patch to add some constraints, while still avoiding an additional table/lookup. The 'port-forward' mode implies that the user wants to avoid any kind of lookup in the datapath (as indicated by the ofctl rule + port-forward mode). I don't see how to completely avoid lookups. IIUC, in this patch there is a match and upcall for the first packet, but there are no matches for subsequent packets. That's right. Allow the first packet to go through match, upcall, dp/cache insertion etc. For subsequent packets avoid lookup. This will work only for flow actions that doesn't modify the packet. If for some reason the flow contains header modifications OVS will not do that correctly because the header is not parsed. Also, if the packet is a bit different from the very first packet, we might attempt to modify headers that doesn't exist. All in all, this is very dangerous and might lead to OVS crash. We can't rely on the user to set specific OF rules for this functionality and we should not have a feature that might crash OVS if not used accurately. The way to not parse the packet at all and to not perform any matches is the way to completely ignore OF rules, but OVS is an OF switch and such functionality just doesn't fit. If I add a constraint to check that there is only one action and it's an OUTPUT action (i.e don't enable port-forward mode if the DP flow contains other actions like modify), like it is done in your patch, that should handle this issue ? Thanks, -Harsha In my change I minimized the lookup as possible to a single 64bit key. And it will actually work with any OF rules and without enabling of any special flags. Would be great to see some performance numbers for it as I didn't see any. With pvp tests (vxlan config), we have seen better performance both in pps: ~50% and cpp: ~35%, at a few thousand flows. Similar improvement can be seen with simple configurations (e.g testpmd in the vm in txonly fwd mode). Besides, I've tried this patch. Maybe I did something wrong (I configured port-forward=true on those ports and those openflow rules, and pinged between those ports). I didn't see it worked (the coverage, and also I added my own prints). When you enable port-forward and start the traffic, you should see a message like this: "dpif_netdev(pmd-c02/id:74)|DBG|Setting port_forward_flow: port: 0x7f63400050b0 flow: 0x7f634000afb0" I'm guessing the flow isn't getting added to the port; the insertion is currently done when there's an emc hit. I should probably move the insertion code to handle_packet_upcall(). As a workaround, can you please update the emc insertion probability (ovs-vsctl --no-wait set Open_vSwitch . other_config:emc-insert-inv-prob=1) and retry your test ? Also, please disable normal mode in the bridge (ovs-ofctl del-flows br0; and then add ofctl rules). Let me know if you still see the problem, I'll work with you offline. With this proposed patch, what will be the behavior in case there are multiple DP flows for that single openflow rule? Right now I'm thinki
Re: [ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
On Tue, Apr 27, 2021 at 4:26 PM Ilya Maximets wrote: > > On 4/27/21 11:56 AM, Sriharsha Basavapatna via dev wrote: > > Hi Eli, > > > > On Sun, Apr 25, 2021 at 6:22 PM Eli Britstein wrote: > >> > >> Hi Harsha, > >> > >> On 4/20/2021 11:07 AM, Sriharsha Basavapatna wrote: > >>> Sometimes a port might be configured with a single flow that just > >>> forwards packets to another port. This would be useful in configs > >>> where the bridge is just fowarding packets between two ports (for > >>> example, between a vhost-user port and a physical port). A flow > >>> that matches only on the in_port and with an action that forwards > >>> to another port would be configured, to avoid learning or matching > >>> on packet headers. > >>> > >>> Example: > >>> $ ovs-ofctl add-flow br0 in_port=1,actions=output:2 > >>> $ ovs-ofctl add-flow br0 in_port=2,actions=output:1 > >>> > >>> This translates to a datapath flow with the match fields wildcarded > >>> for the packet headers. However, the datapath processing still involves > >> > >> There are still several matches (not wildcards): > >> > >>- recirc_id > >>- in_port > >>- packet_type > >>- dl_type > >>- vlan_tci > >>- nw_frag (for ip packets) > >> > >> So there might be multiple flows for each such openflow rule. > >> > >> In the past, I have tried to optimize such scenario, see: > >> > >> https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/357882.html > >> > >> That was wrong as commented afterwards. > >> > >> Another related patch-set was this (also not accepted): > >> > >> https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363948.html > >> > >> Ilya wrote an alternative patch: > >> > >> https://patchwork.ozlabs.org/patch/1105880/ > >> > >> AFAIR, it didn't improve performance either. > > Would be good to have some performance numbers for it as there was > no test results published and I don't know if someone ever tested it. > > > > > Thanks for the above pointers. Ilya had also shared this patch > > recently while discussing this topic at the ovs-dpdk community > > meeting. I want to see if we can utilize part of the logic in that > > patch to add some constraints, while still avoiding an additional > > table/lookup. The 'port-forward' mode implies that the user wants to > > avoid any kind of lookup in the datapath (as indicated by the ofctl > > rule + port-forward mode). > > I don't see how to completely avoid lookups. > > IIUC, in this patch there is a match and upcall for the first packet, > but there are no matches for subsequent packets. That's right. Allow the first packet to go through match, upcall, dp/cache insertion etc. For subsequent packets avoid lookup. > This will work > only for flow actions that doesn't modify the packet. If for some > reason the flow contains header modifications OVS will not do that > correctly because the header is not parsed. Also, if the packet is > a bit different from the very first packet, we might attempt to > modify headers that doesn't exist. All in all, this is very dangerous > and might lead to OVS crash. We can't rely on the user to set specific > OF rules for this functionality and we should not have a feature that > might crash OVS if not used accurately. > > The way to not parse the packet at all and to not perform any matches is > the way to completely ignore OF rules, but OVS is an OF switch and > such functionality just doesn't fit. If I add a constraint to check that there is only one action and it's an OUTPUT action (i.e don't enable port-forward mode if the DP flow contains other actions like modify), like it is done in your patch, that should handle this issue ? Thanks, -Harsha > > In my change I minimized the lookup as possible to a single 64bit key. > And it will actually work with any OF rules and without enabling of > any special flags. Would be great to see some performance numbers > for it as I didn't see any. > > > With pvp tests (vxlan config), we have > > seen better performance both in pps: ~50% and cpp: ~35%, at a few > > thousand flows. Similar improvement can be seen with simple > > configurations (e.g testpmd in the vm in txonly fwd mode). > > > >> > >> Besides, I've tried this patch. Maybe I did something wrong (I > >> configured port-forward=true on those ports and those openflow rules, > >> and pinged between those ports). I didn't see it worked (the coverage, > >> and also I added my own prints). > > > > When you enable port-forward and start the traffic, you should see a > > message like this: > > "dpif_netdev(pmd-c02/id:74)|DBG|Setting port_forward_flow: port: > > 0x7f63400050b0 flow: 0x7f634000afb0" > > > > I'm guessing the flow isn't getting added to the port; the insertion > > is currently done when there's an emc hit. I should probably move the > > insertion code to handle_packet_upcall(). As a workaround, can you > > please update the emc insertion probability (ovs-vsctl --no-wait set > > Open_vSwitch . other_config:emc-insert-inv-prob=1)
Re: [ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
On 4/27/21 11:56 AM, Sriharsha Basavapatna via dev wrote: > Hi Eli, > > On Sun, Apr 25, 2021 at 6:22 PM Eli Britstein wrote: >> >> Hi Harsha, >> >> On 4/20/2021 11:07 AM, Sriharsha Basavapatna wrote: >>> Sometimes a port might be configured with a single flow that just >>> forwards packets to another port. This would be useful in configs >>> where the bridge is just fowarding packets between two ports (for >>> example, between a vhost-user port and a physical port). A flow >>> that matches only on the in_port and with an action that forwards >>> to another port would be configured, to avoid learning or matching >>> on packet headers. >>> >>> Example: >>> $ ovs-ofctl add-flow br0 in_port=1,actions=output:2 >>> $ ovs-ofctl add-flow br0 in_port=2,actions=output:1 >>> >>> This translates to a datapath flow with the match fields wildcarded >>> for the packet headers. However, the datapath processing still involves >> >> There are still several matches (not wildcards): >> >>- recirc_id >>- in_port >>- packet_type >>- dl_type >>- vlan_tci >>- nw_frag (for ip packets) >> >> So there might be multiple flows for each such openflow rule. >> >> In the past, I have tried to optimize such scenario, see: >> >> https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/357882.html >> >> That was wrong as commented afterwards. >> >> Another related patch-set was this (also not accepted): >> >> https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363948.html >> >> Ilya wrote an alternative patch: >> >> https://patchwork.ozlabs.org/patch/1105880/ >> >> AFAIR, it didn't improve performance either. Would be good to have some performance numbers for it as there was no test results published and I don't know if someone ever tested it. > > Thanks for the above pointers. Ilya had also shared this patch > recently while discussing this topic at the ovs-dpdk community > meeting. I want to see if we can utilize part of the logic in that > patch to add some constraints, while still avoiding an additional > table/lookup. The 'port-forward' mode implies that the user wants to > avoid any kind of lookup in the datapath (as indicated by the ofctl > rule + port-forward mode). I don't see how to completely avoid lookups. IIUC, in this patch there is a match and upcall for the first packet, but there are no matches for subsequent packets. This will work only for flow actions that doesn't modify the packet. If for some reason the flow contains header modifications OVS will not do that correctly because the header is not parsed. Also, if the packet is a bit different from the very first packet, we might attempt to modify headers that doesn't exist. All in all, this is very dangerous and might lead to OVS crash. We can't rely on the user to set specific OF rules for this functionality and we should not have a feature that might crash OVS if not used accurately. The way to not parse the packet at all and to not perform any matches is the way to completely ignore OF rules, but OVS is an OF switch and such functionality just doesn't fit. In my change I minimized the lookup as possible to a single 64bit key. And it will actually work with any OF rules and without enabling of any special flags. Would be great to see some performance numbers for it as I didn't see any. > With pvp tests (vxlan config), we have > seen better performance both in pps: ~50% and cpp: ~35%, at a few > thousand flows. Similar improvement can be seen with simple > configurations (e.g testpmd in the vm in txonly fwd mode). > >> >> Besides, I've tried this patch. Maybe I did something wrong (I >> configured port-forward=true on those ports and those openflow rules, >> and pinged between those ports). I didn't see it worked (the coverage, >> and also I added my own prints). > > When you enable port-forward and start the traffic, you should see a > message like this: > "dpif_netdev(pmd-c02/id:74)|DBG|Setting port_forward_flow: port: > 0x7f63400050b0 flow: 0x7f634000afb0" > > I'm guessing the flow isn't getting added to the port; the insertion > is currently done when there's an emc hit. I should probably move the > insertion code to handle_packet_upcall(). As a workaround, can you > please update the emc insertion probability (ovs-vsctl --no-wait set > Open_vSwitch . other_config:emc-insert-inv-prob=1) and retry your test > ? > > Also, please disable normal mode in the bridge (ovs-ofctl del-flows > br0; and then add ofctl rules). Let me know if you still see the > problem, I'll work with you offline. > >> >> With this proposed patch, what will be the behavior in case there are >> multiple DP flows for that single openflow rule? > > Right now I'm thinking that the ofctl rule takes precedence since the > user just wants to forward to another port. If there are multiple DP > flows, then the first one will act as the default flow. What do you > think ? > > Thanks, > -Harsha > > >> >> Thanks, >> Eli >> >>> flow cache (EMC/SMC)
Re: [ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
Hi Eli, On Sun, Apr 25, 2021 at 6:22 PM Eli Britstein wrote: > > Hi Harsha, > > On 4/20/2021 11:07 AM, Sriharsha Basavapatna wrote: > > Sometimes a port might be configured with a single flow that just > > forwards packets to another port. This would be useful in configs > > where the bridge is just fowarding packets between two ports (for > > example, between a vhost-user port and a physical port). A flow > > that matches only on the in_port and with an action that forwards > > to another port would be configured, to avoid learning or matching > > on packet headers. > > > > Example: > > $ ovs-ofctl add-flow br0 in_port=1,actions=output:2 > > $ ovs-ofctl add-flow br0 in_port=2,actions=output:1 > > > > This translates to a datapath flow with the match fields wildcarded > > for the packet headers. However, the datapath processing still involves > > There are still several matches (not wildcards): > >- recirc_id >- in_port >- packet_type >- dl_type >- vlan_tci >- nw_frag (for ip packets) > > So there might be multiple flows for each such openflow rule. > > In the past, I have tried to optimize such scenario, see: > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/357882.html > > That was wrong as commented afterwards. > > Another related patch-set was this (also not accepted): > > https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363948.html > > Ilya wrote an alternative patch: > > https://patchwork.ozlabs.org/patch/1105880/ > > AFAIR, it didn't improve performance either. Thanks for the above pointers. Ilya had also shared this patch recently while discussing this topic at the ovs-dpdk community meeting. I want to see if we can utilize part of the logic in that patch to add some constraints, while still avoiding an additional table/lookup. The 'port-forward' mode implies that the user wants to avoid any kind of lookup in the datapath (as indicated by the ofctl rule + port-forward mode). With pvp tests (vxlan config), we have seen better performance both in pps: ~50% and cpp: ~35%, at a few thousand flows. Similar improvement can be seen with simple configurations (e.g testpmd in the vm in txonly fwd mode). > > Besides, I've tried this patch. Maybe I did something wrong (I > configured port-forward=true on those ports and those openflow rules, > and pinged between those ports). I didn't see it worked (the coverage, > and also I added my own prints). When you enable port-forward and start the traffic, you should see a message like this: "dpif_netdev(pmd-c02/id:74)|DBG|Setting port_forward_flow: port: 0x7f63400050b0 flow: 0x7f634000afb0" I'm guessing the flow isn't getting added to the port; the insertion is currently done when there's an emc hit. I should probably move the insertion code to handle_packet_upcall(). As a workaround, can you please update the emc insertion probability (ovs-vsctl --no-wait set Open_vSwitch . other_config:emc-insert-inv-prob=1) and retry your test ? Also, please disable normal mode in the bridge (ovs-ofctl del-flows br0; and then add ofctl rules). Let me know if you still see the problem, I'll work with you offline. > > With this proposed patch, what will be the behavior in case there are > multiple DP flows for that single openflow rule? Right now I'm thinking that the ofctl rule takes precedence since the user just wants to forward to another port. If there are multiple DP flows, then the first one will act as the default flow. What do you think ? Thanks, -Harsha > > Thanks, > Eli > > > flow cache (EMC/SMC) lookup and with a large number of flows it also > > results in dpcls lookup due to cache miss. Avoiding cache lookup in > > such configurations results in better performance (pps and cpp). > > > > This patch provides a new interface config parameter - "port-forward", > > to avoid datapath cache lookup. When this is enabled, the datapath flow > > is saved in the port when there is a cache hit for the initial packet. > > For subsequent packets, the flow is readily found in the port structure, > > thus avoiding cache and dpcls lookup. > > > > Example: > > $ ovs-vsctl add-port br0 dpdk0 \ > > -- set Interface dpdk0 other_config:port-forward=true > > > > A coverage counter has also been added to track packets processed in > > port-forward mode. > > > > $ ovs-appctl coverage/show | grep datapath_port_forward_packet > > > > Signed-off-by: Sriharsha Basavapatna > > --- > > lib/dpif-netdev.c| 79 ++-- > > vswitchd/vswitch.xml | 11 ++ > > 2 files changed, 72 insertions(+), 18 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index 251788b04..133ed7c1e 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -114,6 +114,7 @@ COVERAGE_DEFINE(datapath_drop_invalid_port); > > COVERAGE_DEFINE(datapath_drop_invalid_bond); > > COVERAGE_DEFINE(datapath_drop_invalid_tnl_port); > > COVERAGE_DEFINE(datapath_drop_rx_invalid_packet); > > +C
Re: [ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
Hi Harsha,
On 4/20/2021 11:07 AM, Sriharsha Basavapatna wrote:
Sometimes a port might be configured with a single flow that just
forwards packets to another port. This would be useful in configs
where the bridge is just fowarding packets between two ports (for
example, between a vhost-user port and a physical port). A flow
that matches only on the in_port and with an action that forwards
to another port would be configured, to avoid learning or matching
on packet headers.
Example:
$ ovs-ofctl add-flow br0 in_port=1,actions=output:2
$ ovs-ofctl add-flow br0 in_port=2,actions=output:1
This translates to a datapath flow with the match fields wildcarded
for the packet headers. However, the datapath processing still involves
There are still several matches (not wildcards):
- recirc_id
- in_port
- packet_type
- dl_type
- vlan_tci
- nw_frag (for ip packets)
So there might be multiple flows for each such openflow rule.
In the past, I have tried to optimize such scenario, see:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-April/357882.html
That was wrong as commented afterwards.
Another related patch-set was this (also not accepted):
https://mail.openvswitch.org/pipermail/ovs-dev/2019-October/363948.html
Ilya wrote an alternative patch:
https://patchwork.ozlabs.org/patch/1105880/
AFAIR, it didn't improve performance either.
Besides, I've tried this patch. Maybe I did something wrong (I
configured port-forward=true on those ports and those openflow rules,
and pinged between those ports). I didn't see it worked (the coverage,
and also I added my own prints).
With this proposed patch, what will be the behavior in case there are
multiple DP flows for that single openflow rule?
Thanks,
Eli
flow cache (EMC/SMC) lookup and with a large number of flows it also
results in dpcls lookup due to cache miss. Avoiding cache lookup in
such configurations results in better performance (pps and cpp).
This patch provides a new interface config parameter - "port-forward",
to avoid datapath cache lookup. When this is enabled, the datapath flow
is saved in the port when there is a cache hit for the initial packet.
For subsequent packets, the flow is readily found in the port structure,
thus avoiding cache and dpcls lookup.
Example:
$ ovs-vsctl add-port br0 dpdk0 \
-- set Interface dpdk0 other_config:port-forward=true
A coverage counter has also been added to track packets processed in
port-forward mode.
$ ovs-appctl coverage/show | grep datapath_port_forward_packet
Signed-off-by: Sriharsha Basavapatna
---
lib/dpif-netdev.c| 79 ++--
vswitchd/vswitch.xml | 11 ++
2 files changed, 72 insertions(+), 18 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 251788b04..133ed7c1e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -114,6 +114,7 @@ COVERAGE_DEFINE(datapath_drop_invalid_port);
COVERAGE_DEFINE(datapath_drop_invalid_bond);
COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+COVERAGE_DEFINE(datapath_port_forward_packet);
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -483,6 +484,7 @@ struct dp_netdev_port {
unsigned *txq_used; /* Number of threads that use each tx queue.
*/
struct ovs_mutex txq_used_mutex;
bool emc_enabled; /* If true EMC will be used. */
+bool port_forward;
char *type; /* Port type as requested by user. */
char *rxq_affinity_list;/* Requested affinity of rx queues. */
};
@@ -557,6 +559,7 @@ struct dp_netdev_flow {
bool dead;
uint32_t mark; /* Unique flow mark assigned to a flow */
+void *port_forward_txp;
/* Statistics. */
struct dp_netdev_flow_stats stats;
@@ -610,6 +613,7 @@ struct polled_queue {
struct dp_netdev_rxq *rxq;
odp_port_t port_no;
bool emc_enabled;
+bool port_forward;
bool rxq_enabled;
uint64_t change_seq;
};
@@ -628,6 +632,7 @@ struct tx_port {
long long last_used;
struct hmap_node node;
long long flush_time;
+struct dp_netdev_flow *port_forward_flow;
struct dp_packet_batch output_pkts;
struct dp_netdev_rxq *output_pkts_rxqs[NETDEV_MAX_BURST];
};
@@ -840,7 +845,8 @@ static void dp_netdev_execute_actions(struct
dp_netdev_pmd_thread *pmd,
const struct nlattr *actions,
size_t actions_len);
static void dp_netdev_input(struct dp_netdev_pmd_thread *,
-struct dp_packet_batch *, odp_port_t port_no);
+struct dp_packet_batch *, odp_port_t port_no,
+bool port_forward);
static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *,
struct dp_packet_batch *);
[ovs-dev] [RFC PATCH] dpif-netdev: Support "port-forward" mode to avoid dp cache lookup
Sometimes a port might be configured with a single flow that just
forwards packets to another port. This would be useful in configs
where the bridge is just fowarding packets between two ports (for
example, between a vhost-user port and a physical port). A flow
that matches only on the in_port and with an action that forwards
to another port would be configured, to avoid learning or matching
on packet headers.
Example:
$ ovs-ofctl add-flow br0 in_port=1,actions=output:2
$ ovs-ofctl add-flow br0 in_port=2,actions=output:1
This translates to a datapath flow with the match fields wildcarded
for the packet headers. However, the datapath processing still involves
flow cache (EMC/SMC) lookup and with a large number of flows it also
results in dpcls lookup due to cache miss. Avoiding cache lookup in
such configurations results in better performance (pps and cpp).
This patch provides a new interface config parameter - "port-forward",
to avoid datapath cache lookup. When this is enabled, the datapath flow
is saved in the port when there is a cache hit for the initial packet.
For subsequent packets, the flow is readily found in the port structure,
thus avoiding cache and dpcls lookup.
Example:
$ ovs-vsctl add-port br0 dpdk0 \
-- set Interface dpdk0 other_config:port-forward=true
A coverage counter has also been added to track packets processed in
port-forward mode.
$ ovs-appctl coverage/show | grep datapath_port_forward_packet
Signed-off-by: Sriharsha Basavapatna
---
lib/dpif-netdev.c| 79 ++--
vswitchd/vswitch.xml | 11 ++
2 files changed, 72 insertions(+), 18 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 251788b04..133ed7c1e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -114,6 +114,7 @@ COVERAGE_DEFINE(datapath_drop_invalid_port);
COVERAGE_DEFINE(datapath_drop_invalid_bond);
COVERAGE_DEFINE(datapath_drop_invalid_tnl_port);
COVERAGE_DEFINE(datapath_drop_rx_invalid_packet);
+COVERAGE_DEFINE(datapath_port_forward_packet);
/* Protects against changes to 'dp_netdevs'. */
static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
@@ -483,6 +484,7 @@ struct dp_netdev_port {
unsigned *txq_used; /* Number of threads that use each tx queue. */
struct ovs_mutex txq_used_mutex;
bool emc_enabled; /* If true EMC will be used. */
+bool port_forward;
char *type; /* Port type as requested by user. */
char *rxq_affinity_list;/* Requested affinity of rx queues. */
};
@@ -557,6 +559,7 @@ struct dp_netdev_flow {
bool dead;
uint32_t mark; /* Unique flow mark assigned to a flow */
+void *port_forward_txp;
/* Statistics. */
struct dp_netdev_flow_stats stats;
@@ -610,6 +613,7 @@ struct polled_queue {
struct dp_netdev_rxq *rxq;
odp_port_t port_no;
bool emc_enabled;
+bool port_forward;
bool rxq_enabled;
uint64_t change_seq;
};
@@ -628,6 +632,7 @@ struct tx_port {
long long last_used;
struct hmap_node node;
long long flush_time;
+struct dp_netdev_flow *port_forward_flow;
struct dp_packet_batch output_pkts;
struct dp_netdev_rxq *output_pkts_rxqs[NETDEV_MAX_BURST];
};
@@ -840,7 +845,8 @@ static void dp_netdev_execute_actions(struct
dp_netdev_pmd_thread *pmd,
const struct nlattr *actions,
size_t actions_len);
static void dp_netdev_input(struct dp_netdev_pmd_thread *,
-struct dp_packet_batch *, odp_port_t port_no);
+struct dp_packet_batch *, odp_port_t port_no,
+bool port_forward);
static void dp_netdev_recirculate(struct dp_netdev_pmd_thread *,
struct dp_packet_batch *);
@@ -2083,6 +2089,7 @@ port_create(const char *devname, const char *type,
port->type = xstrdup(type);
port->sf = NULL;
port->emc_enabled = true;
+port->port_forward = false;
port->need_reconfigure = true;
ovs_mutex_init(&port->txq_used_mutex);
@@ -2845,6 +2852,15 @@ dp_netdev_pmd_remove_flow(struct dp_netdev_pmd_thread
*pmd,
if (flow->mark != INVALID_FLOW_MARK) {
queue_netdev_flow_del(pmd, flow);
}
+if (flow->port_forward_txp) {
+struct tx_port *p = flow->port_forward_txp;
+if (p->port_forward_flow == flow) {
+VLOG_DBG("Deleting port_forward_flow: port: %p flow: %p",
+ p, flow);
+p->port_forward_flow = NULL;
+flow->port_forward_txp = NULL;
+}
+}
flow->dead = true;
dp_netdev_flow_unref(flow);
@@ -3664,6 +3680,7 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread *pmd,
flow->dead = false;
flow->batch = NULL;
flow->mark = INVALID_FLOW_MARK;
+flow->port_forward_txp = NULL;
*CONST_CAST(unsigned *, &flow->pmd_id) = pmd->core_id;
*CONST_C
