On 4/27/2021 6:57 PM, Sriharsha Basavapatna wrote:
On Tue, Apr 27, 2021 at 6:42 PM Eli Britstein<[email protected]>  wrote:
On 4/27/2021 2:45 PM, Sriharsha Basavapatna wrote:
On Tue, Apr 27, 2021 at 4:26 PM Ilya Maximets<[email protected]>  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<[email protected]>  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 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 ?
Indeed this is what the user asked for, but it doesn't fit OVS.

OVS always matches on those fields, so there are multiple DP flows. If
only the first one is hit, it breaks functionality. Though traffic
passes, other flows will age out as their counters will be wrong.
No, other flows won't age out since DP flows won't even get created
for those flows as there will be a hit in dfc_processing() based on
the in_port and the first DP flow would be used (as
p->port_forward_flow is already set; see code below).  The packet/byte
counts for the traffic from those other flows would be counted in the
DP flow corresponding to the first flow (and you would see only one DP
flow in dpctl/dump-flows). I've verified this by running IPv4 and IPv6
traffic parallelly. So it works fine as long as the only ofctl rule
that's specified is for port forwarding (in_port=1, output=2).

             p = pmd_send_port_cache_lookup(pmd, port_no);
             if (p->port_forward_flow) {
                 dp_netdev_queue_batches(packet, p->port_forward_flow,
                                         tcp_flags, batches, n_batches);
Well, this is not exactly "fine". The user sees one DP flow (suppose match on IPv4 type), but IPv6 (and others) traffic hits it.
Another interesting case to test is what if there are OpenFlow rules to
do different things with the "forced" matches, for example drop IPv6,
and forward all the rest.

Will this mode take effect in this case too? If so, it will break also
the desired user configuration, if the first "default" flow is IPv4 one,
and IPv6 will pass.
Yes, this test case could be a problem; so the requirement is there's
only one ofctl rule to just forward all packets (any dl_type) from the
given in_port to another port.
Besides the above, is such requirement enforced by the code?
Thanks,
-Harsha
Am I wrong?

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<[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to