On Wed, Feb 12, 2025 at 4:05 PM Ilya Maximets <[email protected]> wrote: > > On 2/11/25 01:39, Mike Pattrick wrote: > > On Mon, Feb 10, 2025 at 8:30 AM Ilya Maximets <[email protected]> wrote: > >> > >> On 2/7/25 06:46, Mike Pattrick wrote: > >>> On Thu, Feb 6, 2025 at 9:15 AM David Marchand <[email protected]> > >>> wrote: > >>>> > >>>> Hello, > >>>> > >>>> On Wed, Feb 5, 2025 at 1:55 PM Ilya Maximets <[email protected]> wrote: > >>>>> > >>>>> On 1/23/25 16:56, David Marchand wrote: > >>>>>> Rather than drop all pending Tx offloads on recirculation, > >>>>>> preserve inner offloads (and mark packet with outer Tx offloads) > >>>>>> when parsing the packet again. > >>>>>> > >>>>>> Fixes: c6538b443984 ("dpif-netdev: Fix crash due to tunnel offloading > >>>>>> on recirculation.") > >>>>>> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.") > >>>>>> Signed-off-by: David Marchand <[email protected]> > >>>>>> --- > >>>>>> Changes since v1: > >>>>>> - rebased, > >>>>>> - dropped API change on miniflow_extract(), rely on tunnel offloading > >>>>>> flag presence instead, > >>>>>> - introduced dp_packet_reset_outer_offsets, > >>>>>> > >>>>>> --- > >>>>>> lib/dp-packet.h | 23 +++++++++++------------ > >>>>>> lib/dpif-netdev.c | 27 --------------------------- > >>>>>> lib/flow.c | 34 ++++++++++++++++++++++++++++------ > >>>>>> 3 files changed, 39 insertions(+), 45 deletions(-) > >>>>> > >>>>> Hi, David. Thanks for the patch! > >>>>> > >>>>> Did you run some performance tests with this change? It touches the > >>>>> very > >>>>> core of packet parsing, so we need to check how that impacts normal V2V > >>>>> or > >>>>> PVP scenarios even without tunneling. > >>>> > >>>> I would be surprised those added branches add much to the already good > >>>> number of branches in miniflow_extract. > >>>> Though I can understand a concern of decreased performance. > >>>> > >>>> > >>>> I did a "simple" test with testpmd as a tgen and a simple port0 -> > >>>> port1 and port1 -> port0 openflow rules. > >>>> 1 pmd thread per port on isolated cpu, no thread sibling. > >>>> > >>>> I used current main branch: > >>>> 481bc0979 - (HEAD, origin/main, origin/HEAD) route-table: Allow > >>>> parsing routes without nexthop. (7 days ago) <Martin Kalcok> > >>>> > >>>> Unexpectedly, I see a slight improvement (I repeated builds, > >>>> configuration and tests a few times). > >>> > >>> Hello David, > >>> > >>> I also did a few performance tests. In all tests below I generated > >>> traffic in a VM with iperf3, transited a netdev datapath OVS, and > >>> egressed through an i40e network card. All tests were repeated 10 > >>> times and I restarted OVS in between some tests. > >>> > >>> First I tested with tso + tunnel encapsulation with a vxlan tunnel. > >>> > >>> Without patch: > >>> Mean: 6.09 Gbps > >>> Stdev: 0.098 > >>> > >>> With patch: > >>> Mean: 6.20 Gbps > >>> Stdev: 0.097 > >>> > >>> From this it's clear in the tunnel + TSO case there is a noticeable > >>> improvement! > >>> > >>> Next I just tested just a straight path from the VM, through OVS, to the > >>> nic. > >>> > >>> Without patch: > >>> Mean: 16.81 Gbps > >>> Stdev: 0.86 > >>> > >>> With patch: > >>> Mean: 17.68 Gbps > >>> Stdev: 0.91 > >>> > >>> Again we see the small but paradoxical performance improvement with > >>> the patch. There weren't a lot of samples overall, but I ran a t-test > >>> and found a p value of 0.045 suggesting significance. > >>> > >>> Cheers, > >>> M > >>> > >>>> > >>>> > >>>> - testpmd (txonly) mlx5 -> mlx5 OVS mlx5 <-> mlx5 testpmd (mac) > >>>> * Before patch: > >>>> flow-dump from pmd on cpu core: 6 > >>>> ufid:5ba3b6ab-7595-4904-aeb3-410ec10f0f84, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(dpdk1),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=04:3f:72:b2:c0:91/00:00:00:00:00:00,dst=04:3f:72:b2:c0:90/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:100320113, bytes:6420487232, used:0.000s, dp:ovs, > >>>> actions:dpdk0, dp-extra-info:miniflow_bits(4,1) > >>>> flow-dump from pmd on cpu core: 4 > >>>> ufid:3627c676-e0f9-4293-b86b-6824c35f9a6c, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(dpdk0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=04:3f:72:b2:c0:90/00:00:00:00:00:00,dst=02:00:00:00:00:00/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:106807423, bytes:6835675072, used:0.000s, dp:ovs, > >>>> actions:dpdk1, dp-extra-info:miniflow_bits(4,1) > >>>> > >>>> Rx-pps: 11367442 Rx-bps: 5820130688 > >>>> Tx-pps: 11367439 Tx-bps: 5820128800 > >>>> > >>>> * After patch: > >>>> flow-dump from pmd on cpu core: 6 > >>>> ufid:41a51bc1-f6cb-4810-8372-4a9254a1db52, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(dpdk1),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=04:3f:72:b2:c0:91/00:00:00:00:00:00,dst=04:3f:72:b2:c0:90/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:32408002, bytes:2074112128, used:0.000s, dp:ovs, > >>>> actions:dpdk0, dp-extra-info:miniflow_bits(4,1) > >>>> flow-dump from pmd on cpu core: 4 > >>>> ufid:115e4654-1e01-467b-9360-de75eb1e872b, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(dpdk0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=04:3f:72:b2:c0:90/00:00:00:00:00:00,dst=02:00:00:00:00:00/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:37689559, bytes:2412131776, used:0.000s, dp:ovs, > >>>> actions:dpdk1, dp-extra-info:miniflow_bits(4,1) > >>>> > >>>> Rx-pps: 12084135 Rx-bps: 6187077192 > >>>> Tx-pps: 12084135 Tx-bps: 6187077192 > >>>> > >>>> > >>>> - testpmd (txonly) virtio-user -> vhost-user OVS vhost-user -> > >>>> virtio-user testpmd (mac) > >>>> * Before patch: > >>>> flow-dump from pmd on cpu core: 6 > >>>> ufid:79248354-3697-4d2e-9d70-cc4df5602ff9, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(vhost1),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=00:11:22:33:44:56/00:00:00:00:00:00,dst=00:11:22:33:44:55/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:23402111, bytes:1497735104, used:0.000s, dp:ovs, > >>>> actions:vhost0, dp-extra-info:miniflow_bits(4,1) > >>>> flow-dump from pmd on cpu core: 4 > >>>> ufid:ca8974b4-2c7e-49c1-bdc6-5d90638997b6, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(vhost0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=00:11:22:33:44:55/00:00:00:00:00:00,dst=00:11:22:33:44:66/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:23402655, bytes:1497769920, used:0.001s, dp:ovs, > >>>> actions:vhost1, dp-extra-info:miniflow_bits(4,1) > >>>> > >>>> Rx-pps: 6022487 Rx-bps: 3083513840 > >>>> Tx-pps: 6022487 Tx-bps: 3083513840 > >>>> > >>>> * After patch: > >>>> flow-dump from pmd on cpu core: 6 > >>>> ufid:c2bac91a-d8a6-4a96-9d56-aee133d1f047, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(vhost1),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=00:11:22:33:44:56/00:00:00:00:00:00,dst=00:11:22:33:44:55/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:53921535, bytes:3450978240, used:0.000s, dp:ovs, > >>>> actions:vhost0, dp-extra-info:miniflow_bits(4,1) > >>>> flow-dump from pmd on cpu core: 4 > >>>> ufid:c4989fca-2662-4645-8291-8971c00b7cb4, > >>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(vhost0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),packet_type(ns=0,id=0),eth(src=00:11:22:33:44:55/00:00:00:00:00:00,dst=00:11:22:33:44:66/00:00:00:00:00:00),eth_type(0x0800),ipv4(src=198.18.0.1/0.0.0.0,dst=198.18.0.2/0.0.0.0,proto=17/0,tos=0/0,ttl=64/0,frag=no),udp(src=9/0,dst=9/0), > >>>> packets:53921887, bytes:3451000768, used:0.000s, dp:ovs, > >>>> actions:vhost1, dp-extra-info:miniflow_bits(4,1) > >>>> > >>>> Rx-pps: 6042410 Rx-bps: 3093714208 > >>>> Tx-pps: 6042407 Tx-bps: 3093712616 > >>>> > >> > >> Hi, Mike and David. > >> > >> Thanks for the test results, but I don't think they are relevant. At > >> least the > >> David's ones. The datapath flows show no matches on eth addresses and that > >> suggests that the simple match is in use. And miniflow_extract is not > >> called in > >> this case, so the test doesn't really check the changes. The variance in > >> the > >> test results is also concerning as nothing should have changed in the > >> datapath, > >> but the performance changes for some reason. > >> > >> Mike, what OpenFlow rules are you using in your setup? > >> > >> > >> On my end, I did my own set of runs with and without this patch and I see > >> about > >> 0.82% performance degradation for a V2V scenario with a NORMAL OpenFlow > >> rule and > >> no real difference with simple match, which is expected. My numbers are: > >> > >> NORMAL Simple match > >> patch main patch main > >> 7420.0 7481.6 8333.1 8333.9 > >> > >> -0.82 % -0.009 % > >> > >> The numbres are averages over 14 alternating runs of each type, so the > >> results > >> should be statistically significant. The fact that there is no difference > >> in > >> a simple match case also suggests that the difference with NORMAL is real. > >> > >> Could you re-check your tests? > > > > I reran the test as a V2V and confirmed that a normal action was used. > > This time I increased the sample count to 60 runs per branch and > > restarted OVS between each test. Results: > > > > patch main > > mean 1421798 1427197 KBytes/sec > > stdev 35954 35824 > > -0.38 % > > p = 0.41 > > > > High p value > > > > I also ran this test with a ct(),recirc,normal action. > > > > patch main > > mean 1435295 1393243 KBytes/sec > > stdev 33540 36335 > > +3 % > > > > I also threw together a micro benchmark to just time miniflow_extract > > raw: https://gist.github.com/mkp-rh/a47356202a485dac4c838cc07c3b77ba > > > > After 100 samples of 1,000,000 iterations > > > > patch main > > mean 47 41 ns > > stdev 4 2 ns > > -11 % > > Thanks, Mike, for the additional testing. So, it seems there is a trade-off > between some performance decrease in basic V2V cases and a sizeable increase > in cases with the actual recirculation involved. > > So, I'd say it seems fine to me to make that trade as more setups have more > complex pipelines these days. > > What do you think? > > For the backports though, I don't think this should go below 3.5, as sudden > changes in performance may not be desired on stable branches. Also, the > feature is still experimental and it's more of a known behavior rather than > a bug that we drop TSO packets on recirculation.
I think that's reasonable! Cheers, M > > Best regards, Ilya Maximets. > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
