Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
> -Original Message- > From: Flavio Leitner > Sent: Wednesday, July 7, 2021 10:59 PM > To: Ilya Maximets > Cc: Van Haaren, Harry ; Eli Britstein > ; ovs dev ; Ivan Malov > ; Majd Dibbiny > Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > > Hi, > > On Wed, Jul 07, 2021 at 04:53:14PM +0200, Ilya Maximets wrote: > > On 7/6/21 3:34 PM, Van Haaren, Harry wrote: > > >> -Original Message- > > >> From: Ilya Maximets > > >> Sent: Thursday, July 1, 2021 11:32 AM > > >> To: Van Haaren, Harry ; Ilya Maximets > > >> > > >> Cc: Eli Britstein ; ovs dev ; > > >> Ivan > Malov > > >> ; Majd Dibbiny ; Stokes, Ian > > >> ; Ferriter, Cian ; Ben > > >> Pfaff > > >> ; Balazs Nemeth ; Sriharsha > Basavapatna > > >> > > >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > >> > > >> On 6/29/21 1:53 PM, Van Haaren, Harry wrote: > > >>>> -Original Message- > > >>>> From: Ilya Maximets > > >>>> Sent: Monday, June 28, 2021 3:33 PM > > >>>> To: Van Haaren, Harry ; Ilya Maximets > > >>>> ; Sriharsha Basavapatna > > >>>> > > >>>> Cc: Eli Britstein ; ovs dev ; > Ivan > > >> Malov > > >>>> ; Majd Dibbiny ; Stokes, > Ian > > >>>> ; Ferriter, Cian ; Ben > Pfaff > > >>>> ; Balazs Nemeth > > >>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > >>>> > > >>>> On 6/25/21 7:28 PM, Van Haaren, Harry wrote: > > >>>>>> -Original Message- > > >>>>>> From: dev On Behalf Of Ilya > Maximets > > >>>>>> Sent: Friday, June 25, 2021 4:26 PM > > >>>>>> To: Sriharsha Basavapatna ; > Ilya > > >>>> Maximets > > >>>>>> > > >>>>>> Cc: Eli Britstein ; ovs dev ; > Ivan > > >>>> Malov > > >>>>>> ; Majd Dibbiny > > >>>>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > >>>>> > > >>>>> > > >>>>> > > >>>>>>>> That looks good to me. So, I guess, Harsha, we're waiting for > > >>>>>>>> your review/tests here. > > >>>>>>> > > >>>>>>> Thanks Ilya and Eli, looks good to me; I've also tested it and it > > >>>>>>> works > fine. > > >>>>>>> -Harsha > > >>>>>> > > >>>>>> Thanks, everyone. Applied to master. > > >>>>> > > >>>>> Hi Ilya and OVS Community, > > >>>>> > > >>>>> There are open questions around this patchset, why has it been > merged? > > >>>>> > > >>>>> Earlier today, new concerns were raised by Cian around the negative > > >> performance > > >>>> impact of these code changes: > > >>>>> - https://mail.openvswitch.org/pipermail/ovs-dev/2021- > June/384445.html > > >>>>> > > >>>>> Both you (Ilya) and Eli responded, and I was following the > > >>>>> conversation. > Various > > >>>> code changes were suggested, > > >>>>> and some may seem like they might work, Eli mentioned some solutions > might > > >> not > > >>>> work due to the hardware: > > >>>>> I was processing both your comments and input, and planning a > technical reply > > >>>> later today. > > >>>>> - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > > >>>> June/384446.html > > >>>>> - concerns around hw: https://mail.openvswitch.org/pipermail/ovs- > dev/2021- > > >>>> June/384464.html > > >>>> > > >>>> Concerns not really about the hardware, but the API itself > > >>>> that should be clarified a little bit to avoid confusion and > > >>>> avoid incorrect changes like the one I suggested. > > >>>> But this is a small enhancement that could be done on top. > > >>>> > > >>>>> > > >&g
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
Hi, On Wed, Jul 07, 2021 at 04:53:14PM +0200, Ilya Maximets wrote: > On 7/6/21 3:34 PM, Van Haaren, Harry wrote: > >> -Original Message- > >> From: Ilya Maximets > >> Sent: Thursday, July 1, 2021 11:32 AM > >> To: Van Haaren, Harry ; Ilya Maximets > >> > >> Cc: Eli Britstein ; ovs dev ; Ivan > >> Malov > >> ; Majd Dibbiny ; Stokes, Ian > >> ; Ferriter, Cian ; Ben Pfaff > >> ; Balazs Nemeth ; Sriharsha Basavapatna > >> > >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >> > >> On 6/29/21 1:53 PM, Van Haaren, Harry wrote: > >>>> -Original Message- > >>>> From: Ilya Maximets > >>>> Sent: Monday, June 28, 2021 3:33 PM > >>>> To: Van Haaren, Harry ; Ilya Maximets > >>>> ; Sriharsha Basavapatna > >>>> > >>>> Cc: Eli Britstein ; ovs dev ; > >>>> Ivan > >> Malov > >>>> ; Majd Dibbiny ; Stokes, Ian > >>>> ; Ferriter, Cian ; Ben > >>>> Pfaff > >>>> ; Balazs Nemeth > >>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >>>> > >>>> On 6/25/21 7:28 PM, Van Haaren, Harry wrote: > >>>>>> -Original Message- > >>>>>> From: dev On Behalf Of Ilya Maximets > >>>>>> Sent: Friday, June 25, 2021 4:26 PM > >>>>>> To: Sriharsha Basavapatna ; Ilya > >>>> Maximets > >>>>>> > >>>>>> Cc: Eli Britstein ; ovs dev ; > >>>>>> Ivan > >>>> Malov > >>>>>> ; Majd Dibbiny > >>>>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >>>>> > >>>>> > >>>>> > >>>>>>>> That looks good to me. So, I guess, Harsha, we're waiting for > >>>>>>>> your review/tests here. > >>>>>>> > >>>>>>> Thanks Ilya and Eli, looks good to me; I've also tested it and it > >>>>>>> works fine. > >>>>>>> -Harsha > >>>>>> > >>>>>> Thanks, everyone. Applied to master. > >>>>> > >>>>> Hi Ilya and OVS Community, > >>>>> > >>>>> There are open questions around this patchset, why has it been merged? > >>>>> > >>>>> Earlier today, new concerns were raised by Cian around the negative > >> performance > >>>> impact of these code changes: > >>>>> - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html > >>>>> > >>>>> Both you (Ilya) and Eli responded, and I was following the > >>>>> conversation. Various > >>>> code changes were suggested, > >>>>> and some may seem like they might work, Eli mentioned some solutions > >>>>> might > >> not > >>>> work due to the hardware: > >>>>> I was processing both your comments and input, and planning a technical > >>>>> reply > >>>> later today. > >>>>> - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > >>>> June/384446.html > >>>>> - concerns around hw: > >>>>> https://mail.openvswitch.org/pipermail/ovs-dev/2021- > >>>> June/384464.html > >>>> > >>>> Concerns not really about the hardware, but the API itself > >>>> that should be clarified a little bit to avoid confusion and > >>>> avoid incorrect changes like the one I suggested. > >>>> But this is a small enhancement that could be done on top. > >>>> > >>>>> > >>>>> Keep in mind that there are open performance issues to be worked out, > >>>>> that > >> have > >>>> not been resolved at this point in the conversation. > >>>> > >>>> Performance issue that can be worked out, will be worked out > >>>> in a separate patch , v1 for which we already have on a mailing > >>>> list for some time, so it didn't make sense to to re-validate > >>>> the whole series again due to this one pretty obvious change. > >>>> > >>>>> There is no agreement on solution
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
On 7/6/21 3:34 PM, Van Haaren, Harry wrote: >> -Original Message- >> From: Ilya Maximets >> Sent: Thursday, July 1, 2021 11:32 AM >> To: Van Haaren, Harry ; Ilya Maximets >> >> Cc: Eli Britstein ; ovs dev ; Ivan >> Malov >> ; Majd Dibbiny ; Stokes, Ian >> ; Ferriter, Cian ; Ben Pfaff >> ; Balazs Nemeth ; Sriharsha Basavapatna >> >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload >> >> On 6/29/21 1:53 PM, Van Haaren, Harry wrote: >>>> -Original Message- >>>> From: Ilya Maximets >>>> Sent: Monday, June 28, 2021 3:33 PM >>>> To: Van Haaren, Harry ; Ilya Maximets >>>> ; Sriharsha Basavapatna >>>> >>>> Cc: Eli Britstein ; ovs dev ; Ivan >> Malov >>>> ; Majd Dibbiny ; Stokes, Ian >>>> ; Ferriter, Cian ; Ben Pfaff >>>> ; Balazs Nemeth >>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload >>>> >>>> On 6/25/21 7:28 PM, Van Haaren, Harry wrote: >>>>>> -Original Message----- >>>>>> From: dev On Behalf Of Ilya Maximets >>>>>> Sent: Friday, June 25, 2021 4:26 PM >>>>>> To: Sriharsha Basavapatna ; Ilya >>>> Maximets >>>>>> >>>>>> Cc: Eli Britstein ; ovs dev ; >>>>>> Ivan >>>> Malov >>>>>> ; Majd Dibbiny >>>>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload >>>>> >>>>> >>>>> >>>>>>>> That looks good to me. So, I guess, Harsha, we're waiting for >>>>>>>> your review/tests here. >>>>>>> >>>>>>> Thanks Ilya and Eli, looks good to me; I've also tested it and it works >>>>>>> fine. >>>>>>> -Harsha >>>>>> >>>>>> Thanks, everyone. Applied to master. >>>>> >>>>> Hi Ilya and OVS Community, >>>>> >>>>> There are open questions around this patchset, why has it been merged? >>>>> >>>>> Earlier today, new concerns were raised by Cian around the negative >> performance >>>> impact of these code changes: >>>>> - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html >>>>> >>>>> Both you (Ilya) and Eli responded, and I was following the conversation. >>>>> Various >>>> code changes were suggested, >>>>> and some may seem like they might work, Eli mentioned some solutions might >> not >>>> work due to the hardware: >>>>> I was processing both your comments and input, and planning a technical >>>>> reply >>>> later today. >>>>> - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021- >>>> June/384446.html >>>>> - concerns around hw: https://mail.openvswitch.org/pipermail/ovs-dev/2021- >>>> June/384464.html >>>> >>>> Concerns not really about the hardware, but the API itself >>>> that should be clarified a little bit to avoid confusion and >>>> avoid incorrect changes like the one I suggested. >>>> But this is a small enhancement that could be done on top. >>>> >>>>> >>>>> Keep in mind that there are open performance issues to be worked out, that >> have >>>> not been resolved at this point in the conversation. >>>> >>>> Performance issue that can be worked out, will be worked out >>>> in a separate patch , v1 for which we already have on a mailing >>>> list for some time, so it didn't make sense to to re-validate >>>> the whole series again due to this one pretty obvious change. >>>> >>>>> There is no agreement on solutions, nor an agreement to ignore the >> performance >>>> degradation, or to try resolve this degradation later. >>>> >>>> Particular part of the packet restoration call seems hard >>>> to avoid in a long term (I don't see a good solution for that), >>>> but the short term solution might be implemented on top. >>>> The part with multiple reads of recirc_id and checking if >>>> offloading is enabled has a fix already (that needs a v2, but >>>> anyway). >>>> >>>>> >>>>> That these patches have been merged is i
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
> -Original Message- > From: Ilya Maximets > Sent: Thursday, July 1, 2021 11:32 AM > To: Van Haaren, Harry ; Ilya Maximets > > Cc: Eli Britstein ; ovs dev ; Ivan > Malov > ; Majd Dibbiny ; Stokes, Ian > ; Ferriter, Cian ; Ben Pfaff > ; Balazs Nemeth ; Sriharsha Basavapatna > > Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > On 6/29/21 1:53 PM, Van Haaren, Harry wrote: > >> -Original Message- > >> From: Ilya Maximets > >> Sent: Monday, June 28, 2021 3:33 PM > >> To: Van Haaren, Harry ; Ilya Maximets > >> ; Sriharsha Basavapatna > >> > >> Cc: Eli Britstein ; ovs dev ; Ivan > Malov > >> ; Majd Dibbiny ; Stokes, Ian > >> ; Ferriter, Cian ; Ben Pfaff > >> ; Balazs Nemeth > >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >> > >> On 6/25/21 7:28 PM, Van Haaren, Harry wrote: > >>>> -Original Message- > >>>> From: dev On Behalf Of Ilya Maximets > >>>> Sent: Friday, June 25, 2021 4:26 PM > >>>> To: Sriharsha Basavapatna ; Ilya > >> Maximets > >>>> > >>>> Cc: Eli Britstein ; ovs dev ; > >>>> Ivan > >> Malov > >>>> ; Majd Dibbiny > >>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >>> > >>> > >>> > >>>>>> That looks good to me. So, I guess, Harsha, we're waiting for > >>>>>> your review/tests here. > >>>>> > >>>>> Thanks Ilya and Eli, looks good to me; I've also tested it and it works > >>>>> fine. > >>>>> -Harsha > >>>> > >>>> Thanks, everyone. Applied to master. > >>> > >>> Hi Ilya and OVS Community, > >>> > >>> There are open questions around this patchset, why has it been merged? > >>> > >>> Earlier today, new concerns were raised by Cian around the negative > performance > >> impact of these code changes: > >>> - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html > >>> > >>> Both you (Ilya) and Eli responded, and I was following the conversation. > >>> Various > >> code changes were suggested, > >>> and some may seem like they might work, Eli mentioned some solutions might > not > >> work due to the hardware: > >>> I was processing both your comments and input, and planning a technical > >>> reply > >> later today. > >>> - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > >> June/384446.html > >>> - concerns around hw: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > >> June/384464.html > >> > >> Concerns not really about the hardware, but the API itself > >> that should be clarified a little bit to avoid confusion and > >> avoid incorrect changes like the one I suggested. > >> But this is a small enhancement that could be done on top. > >> > >>> > >>> Keep in mind that there are open performance issues to be worked out, that > have > >> not been resolved at this point in the conversation. > >> > >> Performance issue that can be worked out, will be worked out > >> in a separate patch , v1 for which we already have on a mailing > >> list for some time, so it didn't make sense to to re-validate > >> the whole series again due to this one pretty obvious change. > >> > >>> There is no agreement on solutions, nor an agreement to ignore the > performance > >> degradation, or to try resolve this degradation later. > >> > >> Particular part of the packet restoration call seems hard > >> to avoid in a long term (I don't see a good solution for that), > >> but the short term solution might be implemented on top. > >> The part with multiple reads of recirc_id and checking if > >> offloading is enabled has a fix already (that needs a v2, but > >> anyway). > >> > >>> > >>> That these patches have been merged is inappropriate: > >>> 1) Not enough time given for responses (11 am concerns raised, 5pm merged > >> without resolution? (Irish timezone)) > >> > >> I responded with suggestions and arguments against solutions > >> suggested in the report, Eli responded with rejection of one > >> one of my suggestions. And it s
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
On 7/1/21 6:44 PM, Van Haaren, Harry wrote: > I'm Out Of Office tomorrow Friday 2nd July, but will reply to this thread > early > next week when back in the office. Sure. 5th and 6th are public holidays for me, though. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
> -Original Message- > From: Ilya Maximets > Sent: Thursday, July 1, 2021 11:32 AM > To: Van Haaren, Harry ; Ilya Maximets > > Cc: Eli Britstein ; ovs dev ; Ivan > Malov > ; Majd Dibbiny ; Stokes, Ian > ; Ferriter, Cian ; Ben Pfaff > ; Balazs Nemeth ; Sriharsha Basavapatna > > Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > On 6/29/21 1:53 PM, Van Haaren, Harry wrote: > >> -Original Message- > >> From: Ilya Maximets > >> Sent: Monday, June 28, 2021 3:33 PM > >> To: Van Haaren, Harry ; Ilya Maximets > >> ; Sriharsha Basavapatna > >> > >> Cc: Eli Britstein ; ovs dev ; Ivan > Malov > >> ; Majd Dibbiny ; Stokes, Ian > >> ; Ferriter, Cian ; Ben Pfaff > >> ; Balazs Nemeth > >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >> > >> On 6/25/21 7:28 PM, Van Haaren, Harry wrote: > >>>> -Original Message- > >>>> From: dev On Behalf Of Ilya Maximets > >>>> Sent: Friday, June 25, 2021 4:26 PM > >>>> To: Sriharsha Basavapatna ; Ilya > >> Maximets > >>>> > >>>> Cc: Eli Britstein ; ovs dev ; > >>>> Ivan > >> Malov > >>>> ; Majd Dibbiny > >>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >>> > >>> > >>> > >>>>>> That looks good to me. So, I guess, Harsha, we're waiting for > >>>>>> your review/tests here. > >>>>> > >>>>> Thanks Ilya and Eli, looks good to me; I've also tested it and it works > >>>>> fine. > >>>>> -Harsha > >>>> > >>>> Thanks, everyone. Applied to master. > >>> > >>> Hi Ilya and OVS Community, > >>> > >>> There are open questions around this patchset, why has it been merged? > >>> > >>> Earlier today, new concerns were raised by Cian around the negative > performance > >> impact of these code changes: > >>> - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html > >>> > >>> Both you (Ilya) and Eli responded, and I was following the conversation. > >>> Various > >> code changes were suggested, > >>> and some may seem like they might work, Eli mentioned some solutions might > not > >> work due to the hardware: > >>> I was processing both your comments and input, and planning a technical > >>> reply > >> later today. > >>> - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > >> June/384446.html > >>> - concerns around hw: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > >> June/384464.html > >> > >> Concerns not really about the hardware, but the API itself > >> that should be clarified a little bit to avoid confusion and > >> avoid incorrect changes like the one I suggested. > >> But this is a small enhancement that could be done on top. > >> > >>> > >>> Keep in mind that there are open performance issues to be worked out, that > have > >> not been resolved at this point in the conversation. > >> > >> Performance issue that can be worked out, will be worked out > >> in a separate patch , v1 for which we already have on a mailing > >> list for some time, so it didn't make sense to to re-validate > >> the whole series again due to this one pretty obvious change. > >> > >>> There is no agreement on solutions, nor an agreement to ignore the > performance > >> degradation, or to try resolve this degradation later. > >> > >> Particular part of the packet restoration call seems hard > >> to avoid in a long term (I don't see a good solution for that), > >> but the short term solution might be implemented on top. > >> The part with multiple reads of recirc_id and checking if > >> offloading is enabled has a fix already (that needs a v2, but > >> anyway). > >> > >>> > >>> That these patches have been merged is inappropriate: > >>> 1) Not enough time given for responses (11 am concerns raised, 5pm merged > >> without resolution? (Irish timezone)) > >> > >> I responded with suggestions and arguments against solutions > >> suggested in the report, Eli responded with rejection of one > >> one of my suggestions. And it s
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
On 6/29/21 1:53 PM, Van Haaren, Harry wrote: >> -Original Message- >> From: Ilya Maximets >> Sent: Monday, June 28, 2021 3:33 PM >> To: Van Haaren, Harry ; Ilya Maximets >> ; Sriharsha Basavapatna >> >> Cc: Eli Britstein ; ovs dev ; Ivan >> Malov >> ; Majd Dibbiny ; Stokes, Ian >> ; Ferriter, Cian ; Ben Pfaff >> ; Balazs Nemeth >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload >> >> On 6/25/21 7:28 PM, Van Haaren, Harry wrote: >>>> -Original Message- >>>> From: dev On Behalf Of Ilya Maximets >>>> Sent: Friday, June 25, 2021 4:26 PM >>>> To: Sriharsha Basavapatna ; Ilya >> Maximets >>>> >>>> Cc: Eli Britstein ; ovs dev ; Ivan >> Malov >>>> ; Majd Dibbiny >>>> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload >>> >>> >>> >>>>>> That looks good to me. So, I guess, Harsha, we're waiting for >>>>>> your review/tests here. >>>>> >>>>> Thanks Ilya and Eli, looks good to me; I've also tested it and it works >>>>> fine. >>>>> -Harsha >>>> >>>> Thanks, everyone. Applied to master. >>> >>> Hi Ilya and OVS Community, >>> >>> There are open questions around this patchset, why has it been merged? >>> >>> Earlier today, new concerns were raised by Cian around the negative >>> performance >> impact of these code changes: >>> - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html >>> >>> Both you (Ilya) and Eli responded, and I was following the conversation. >>> Various >> code changes were suggested, >>> and some may seem like they might work, Eli mentioned some solutions might >>> not >> work due to the hardware: >>> I was processing both your comments and input, and planning a technical >>> reply >> later today. >>> - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021- >> June/384446.html >>> - concerns around hw: https://mail.openvswitch.org/pipermail/ovs-dev/2021- >> June/384464.html >> >> Concerns not really about the hardware, but the API itself >> that should be clarified a little bit to avoid confusion and >> avoid incorrect changes like the one I suggested. >> But this is a small enhancement that could be done on top. >> >>> >>> Keep in mind that there are open performance issues to be worked out, that >>> have >> not been resolved at this point in the conversation. >> >> Performance issue that can be worked out, will be worked out >> in a separate patch , v1 for which we already have on a mailing >> list for some time, so it didn't make sense to to re-validate >> the whole series again due to this one pretty obvious change. >> >>> There is no agreement on solutions, nor an agreement to ignore the >>> performance >> degradation, or to try resolve this degradation later. >> >> Particular part of the packet restoration call seems hard >> to avoid in a long term (I don't see a good solution for that), >> but the short term solution might be implemented on top. >> The part with multiple reads of recirc_id and checking if >> offloading is enabled has a fix already (that needs a v2, but >> anyway). >> >>> >>> That these patches have been merged is inappropriate: >>> 1) Not enough time given for responses (11 am concerns raised, 5pm merged >> without resolution? (Irish timezone)) >> >> I responded with suggestions and arguments against solutions >> suggested in the report, Eli responded with rejection of one >> one of my suggestions. And it seems clear (for me) that >> there is no good solution for this part at the moment. >> Part of the performance could be won back, but the rest >> seems to be inevitable. As a short-term solution we can >> guard the netdev_hw_miss_packet_recover() with experimental >> API ifdef, but it will strike back anyway in the future. >> >>> 2) Open question not addressed/resolved, resulting in a 6% known negative >> performance impact being merged. >> >> I don't think it wasn't addressed. > > Was code merged that resulted in a known regression of 6%? Yes. Facts are > facts. > I don't care for arguing over exactly what "addressed" means in this context. > > >>> 3) Suggestions provided were not reviewed tech
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
> -Original Message- > From: Ilya Maximets > Sent: Monday, June 28, 2021 3:33 PM > To: Van Haaren, Harry ; Ilya Maximets > ; Sriharsha Basavapatna > > Cc: Eli Britstein ; ovs dev ; Ivan > Malov > ; Majd Dibbiny ; Stokes, Ian > ; Ferriter, Cian ; Ben Pfaff > ; Balazs Nemeth > Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > On 6/25/21 7:28 PM, Van Haaren, Harry wrote: > >> -Original Message- > >> From: dev On Behalf Of Ilya Maximets > >> Sent: Friday, June 25, 2021 4:26 PM > >> To: Sriharsha Basavapatna ; Ilya > Maximets > >> > >> Cc: Eli Britstein ; ovs dev ; Ivan > Malov > >> ; Majd Dibbiny > >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > > > > > > >>>> That looks good to me. So, I guess, Harsha, we're waiting for > >>>> your review/tests here. > >>> > >>> Thanks Ilya and Eli, looks good to me; I've also tested it and it works > >>> fine. > >>> -Harsha > >> > >> Thanks, everyone. Applied to master. > > > > Hi Ilya and OVS Community, > > > > There are open questions around this patchset, why has it been merged? > > > > Earlier today, new concerns were raised by Cian around the negative > > performance > impact of these code changes: > > - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html > > > > Both you (Ilya) and Eli responded, and I was following the conversation. > > Various > code changes were suggested, > > and some may seem like they might work, Eli mentioned some solutions might > > not > work due to the hardware: > > I was processing both your comments and input, and planning a technical > > reply > later today. > > - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > June/384446.html > > - concerns around hw: https://mail.openvswitch.org/pipermail/ovs-dev/2021- > June/384464.html > > Concerns not really about the hardware, but the API itself > that should be clarified a little bit to avoid confusion and > avoid incorrect changes like the one I suggested. > But this is a small enhancement that could be done on top. > > > > > Keep in mind that there are open performance issues to be worked out, that > > have > not been resolved at this point in the conversation. > > Performance issue that can be worked out, will be worked out > in a separate patch , v1 for which we already have on a mailing > list for some time, so it didn't make sense to to re-validate > the whole series again due to this one pretty obvious change. > > > There is no agreement on solutions, nor an agreement to ignore the > > performance > degradation, or to try resolve this degradation later. > > Particular part of the packet restoration call seems hard > to avoid in a long term (I don't see a good solution for that), > but the short term solution might be implemented on top. > The part with multiple reads of recirc_id and checking if > offloading is enabled has a fix already (that needs a v2, but > anyway). > > > > > That these patches have been merged is inappropriate: > > 1) Not enough time given for responses (11 am concerns raised, 5pm merged > without resolution? (Irish timezone)) > > I responded with suggestions and arguments against solutions > suggested in the report, Eli responded with rejection of one > one of my suggestions. And it seems clear (for me) that > there is no good solution for this part at the moment. > Part of the performance could be won back, but the rest > seems to be inevitable. As a short-term solution we can > guard the netdev_hw_miss_packet_recover() with experimental > API ifdef, but it will strike back anyway in the future. > > > 2) Open question not addressed/resolved, resulting in a 6% known negative > performance impact being merged. > > I don't think it wasn't addressed. Was code merged that resulted in a known regression of 6%? Yes. Facts are facts. I don't care for arguing over exactly what "addressed" means in this context. > > 3) Suggestions provided were not reviewed technically in detail (no > > technical > collaboration or code-changes/patches reviewed) > > Patches was heavily reviewed/tested by at least 4 different > parties including 2 test rounds from Intel engineers that, > I believe, included testing of partial offloading. And that > bothers me the most. If I can not trust performance test > reports, I'm not sure performance can be a gating factor here. >
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
On 6/25/21 7:28 PM, Van Haaren, Harry wrote: >> -Original Message- >> From: dev On Behalf Of Ilya Maximets >> Sent: Friday, June 25, 2021 4:26 PM >> To: Sriharsha Basavapatna ; Ilya Maximets >> >> Cc: Eli Britstein ; ovs dev ; Ivan >> Malov >> ; Majd Dibbiny >> Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > > > >>>> That looks good to me. So, I guess, Harsha, we're waiting for >>>> your review/tests here. >>> >>> Thanks Ilya and Eli, looks good to me; I've also tested it and it works >>> fine. >>> -Harsha >> >> Thanks, everyone. Applied to master. > > Hi Ilya and OVS Community, > > There are open questions around this patchset, why has it been merged? > > Earlier today, new concerns were raised by Cian around the negative > performance impact of these code changes: > - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html > > Both you (Ilya) and Eli responded, and I was following the conversation. > Various code changes were suggested, > and some may seem like they might work, Eli mentioned some solutions might > not work due to the hardware: > I was processing both your comments and input, and planning a technical reply > later today. > - suggestions: > https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384446.html > - concerns around hw: > https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384464.html Concerns not really about the hardware, but the API itself that should be clarified a little bit to avoid confusion and avoid incorrect changes like the one I suggested. But this is a small enhancement that could be done on top. > > Keep in mind that there are open performance issues to be worked out, that > have not been resolved at this point in the conversation. Performance issue that can be worked out, will be worked out in a separate patch , v1 for which we already have on a mailing list for some time, so it didn't make sense to to re-validate the whole series again due to this one pretty obvious change. > There is no agreement on solutions, nor an agreement to ignore the > performance degradation, or to try resolve this degradation later. Particular part of the packet restoration call seems hard to avoid in a long term (I don't see a good solution for that), but the short term solution might be implemented on top. The part with multiple reads of recirc_id and checking if offloading is enabled has a fix already (that needs a v2, but anyway). > > That these patches have been merged is inappropriate: > 1) Not enough time given for responses (11 am concerns raised, 5pm merged > without resolution? (Irish timezone)) I responded with suggestions and arguments against solutions suggested in the report, Eli responded with rejection of one one of my suggestions. And it seems clear (for me) that there is no good solution for this part at the moment. Part of the performance could be won back, but the rest seems to be inevitable. As a short-term solution we can guard the netdev_hw_miss_packet_recover() with experimental API ifdef, but it will strike back anyway in the future. > 2) Open question not addressed/resolved, resulting in a 6% known negative > performance impact being merged. I don't think it wasn't addressed. > 3) Suggestions provided were not reviewed technically in detail (no technical > collaboration or code-changes/patches reviewed) Patches was heavily reviewed/tested by at least 4 different parties including 2 test rounds from Intel engineers that, I believe, included testing of partial offloading. And that bothers me the most. If I can not trust performance test reports, I'm not sure performance can be a gating factor here. > > I feel that the OVS process of allowing time for community review and > collaboration was not adhered to in this instance. > As a result, code was merged that is known to cause performance degradation. > > Therefore, this email is a request to revert these patches as they are not > currently fit for inclusion in my opinion. > > As next steps, I can propose the following: > 1) Revert the patches from master branch > 2) Continue technical discussion on how to avoid negative performance impact > 3) Review solutions, allowing time for responses and replies > 4) Merge a future revision of this patchset at a later date I don't think that there is a necessity to revert the patch-set. All performance issues can be addressed by small fixes on top: 1. Patch from Balazs to read certain information per-batch. 2. Optionally, guard packet miss recovery with ifdef of an experimental API. -- I'm not sure about this one as it will strike back in the future. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
> -Original Message- > From: dev On Behalf Of Ilya Maximets > Sent: Friday, June 25, 2021 4:26 PM > To: Sriharsha Basavapatna ; Ilya Maximets > > Cc: Eli Britstein ; ovs dev ; Ivan > Malov > ; Majd Dibbiny > Subject: Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload > >> That looks good to me. So, I guess, Harsha, we're waiting for > >> your review/tests here. > > > > Thanks Ilya and Eli, looks good to me; I've also tested it and it works > > fine. > > -Harsha > > Thanks, everyone. Applied to master. Hi Ilya and OVS Community, There are open questions around this patchset, why has it been merged? Earlier today, new concerns were raised by Cian around the negative performance impact of these code changes: - https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384445.html Both you (Ilya) and Eli responded, and I was following the conversation. Various code changes were suggested, and some may seem like they might work, Eli mentioned some solutions might not work due to the hardware: I was processing both your comments and input, and planning a technical reply later today. - suggestions: https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384446.html - concerns around hw: https://mail.openvswitch.org/pipermail/ovs-dev/2021-June/384464.html Keep in mind that there are open performance issues to be worked out, that have not been resolved at this point in the conversation. There is no agreement on solutions, nor an agreement to ignore the performance degradation, or to try resolve this degradation later. That these patches have been merged is inappropriate: 1) Not enough time given for responses (11 am concerns raised, 5pm merged without resolution? (Irish timezone)) 2) Open question not addressed/resolved, resulting in a 6% known negative performance impact being merged. 3) Suggestions provided were not reviewed technically in detail (no technical collaboration or code-changes/patches reviewed) I feel that the OVS process of allowing time for community review and collaboration was not adhered to in this instance. As a result, code was merged that is known to cause performance degradation. Therefore, this email is a request to revert these patches as they are not currently fit for inclusion in my opinion. As next steps, I can propose the following: 1) Revert the patches from master branch 2) Continue technical discussion on how to avoid negative performance impact 3) Review solutions, allowing time for responses and replies 4) Merge a future revision of this patchset at a later date > Best regards, Ilya Maximets. Regards, -Harry ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
On 6/24/21 8:18 AM, Sriharsha Basavapatna wrote:
> On Thu, Jun 24, 2021 at 12:23 AM Ilya Maximets wrote:
>>
>> On 6/23/21 5:52 PM, Eli Britstein wrote:
>>> VXLAN decap in OVS-DPDK configuration consists of two flows:
>>> F1: in_port(ens1f0),eth(),ipv4(),udp(), actions:tnl_pop(vxlan_sys_4789)
>>> F2: tunnel(),in_port(vxlan_sys_4789),eth(),ipv4(), actions:ens1f0_0
>>>
>>> F1 is a classification flow. It has outer headers matches and it
>>> classifies the packet as a VXLAN packet, and using tnl_pop action the
>>> packet continues processing in F2.
>>> F2 is a flow that has matches on tunnel metadata as well as on the inner
>>> packet headers (as any other flow).
>>>
>>> In order to fully offload VXLAN decap path, both F1 and F2 should be
>>> offloaded. As there are more than one flow in HW, it is possible that
>>> F1 is done by HW but F2 is not. Packet is received by SW, and should be
>>> processed starting from F2 as F1 was already done by HW.
>>> Rte_flows are applicable only on physical port IDs. Keeping the original
>>> physical in port on which the packet is received on enables applying
>>> vport flows (e.g. F2) on that physical port.
>>>
>>> This patch-set makes use of [1] introduced in DPDK 20.11, that adds API
>>> for tunnel offloads.
>>>
>>> Note that MLX5 PMD has a bug that the tnl_pop private actions must be
>>> first. In OVS it is not.
>>> Fixing this issue is scheduled for 21.05 (and stable 20.11.2).
>>> Meanwhile, tests were done with a workaround for it [2].
>>>
>>> v2-v1:
>>> - Tracking original in_port, and applying vport on that physical port
>>> instead of all PFs.
>>> v3-v2:
>>> - Traversing ports using a new API instead of flow_dump.
>>> - Refactor packet state recover logic, with bug fix for error pop_header.
>>> - One ref count for netdev in non-tunnel case.
>>> - Rename variables, comments, rebase.
>>> v4-v3:
>>> - Extract orig_in_port from physdev for flow modify.
>>> - Miss handling fixes.
>>> v5-v4:
>>> - Drop refactor offload rule creation commit.
>>> - Comment about setting in_port in restore.
>>> - Refactor vports flow offload commit.
>>> v6-v5:
>>> - Fixed duplicate netdev ref bug.
>>> v7-v6:
>>> - Adopting Ilya's diff, with a minor fix in set_error stub.
>>> - Fixed abort (remove OVS_NOT_REACHED()) with tunnels other than vxlan
>>> ("netdev-offload-dpdk: Support tunnel pop action.").
>>
>> Thanks! I see the only difference (beside the set_error fix) with what
>> I have locally is following:
>>
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index 363f32f71..6bd5b6c9f 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -835,7 +835,9 @@ vport_to_rte_tunnel(struct netdev *vport,
>>netdev_dpdk_get_port_id(netdev));
>> }
>> } else {
>> -OVS_NOT_REACHED();
>> +VLOG_DBG_RL(&rl, "vport type '%s' is not supported",
>> +netdev_get_type(vport));
>> +return -1;
>> }
>>
>> return 0;
>> ---
>>
>> That looks good to me. So, I guess, Harsha, we're waiting for
>> your review/tests here.
>
> Thanks Ilya and Eli, looks good to me; I've also tested it and it works fine.
> -Harsha
Thanks, everyone. Applied to master.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
On Thu, Jun 24, 2021 at 12:23 AM Ilya Maximets wrote:
>
> On 6/23/21 5:52 PM, Eli Britstein wrote:
> > VXLAN decap in OVS-DPDK configuration consists of two flows:
> > F1: in_port(ens1f0),eth(),ipv4(),udp(), actions:tnl_pop(vxlan_sys_4789)
> > F2: tunnel(),in_port(vxlan_sys_4789),eth(),ipv4(), actions:ens1f0_0
> >
> > F1 is a classification flow. It has outer headers matches and it
> > classifies the packet as a VXLAN packet, and using tnl_pop action the
> > packet continues processing in F2.
> > F2 is a flow that has matches on tunnel metadata as well as on the inner
> > packet headers (as any other flow).
> >
> > In order to fully offload VXLAN decap path, both F1 and F2 should be
> > offloaded. As there are more than one flow in HW, it is possible that
> > F1 is done by HW but F2 is not. Packet is received by SW, and should be
> > processed starting from F2 as F1 was already done by HW.
> > Rte_flows are applicable only on physical port IDs. Keeping the original
> > physical in port on which the packet is received on enables applying
> > vport flows (e.g. F2) on that physical port.
> >
> > This patch-set makes use of [1] introduced in DPDK 20.11, that adds API
> > for tunnel offloads.
> >
> > Note that MLX5 PMD has a bug that the tnl_pop private actions must be
> > first. In OVS it is not.
> > Fixing this issue is scheduled for 21.05 (and stable 20.11.2).
> > Meanwhile, tests were done with a workaround for it [2].
> >
> > v2-v1:
> > - Tracking original in_port, and applying vport on that physical port
> > instead of all PFs.
> > v3-v2:
> > - Traversing ports using a new API instead of flow_dump.
> > - Refactor packet state recover logic, with bug fix for error pop_header.
> > - One ref count for netdev in non-tunnel case.
> > - Rename variables, comments, rebase.
> > v4-v3:
> > - Extract orig_in_port from physdev for flow modify.
> > - Miss handling fixes.
> > v5-v4:
> > - Drop refactor offload rule creation commit.
> > - Comment about setting in_port in restore.
> > - Refactor vports flow offload commit.
> > v6-v5:
> > - Fixed duplicate netdev ref bug.
> > v7-v6:
> > - Adopting Ilya's diff, with a minor fix in set_error stub.
> > - Fixed abort (remove OVS_NOT_REACHED()) with tunnels other than vxlan
> > ("netdev-offload-dpdk: Support tunnel pop action.").
>
> Thanks! I see the only difference (beside the set_error fix) with what
> I have locally is following:
>
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index 363f32f71..6bd5b6c9f 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -835,7 +835,9 @@ vport_to_rte_tunnel(struct netdev *vport,
>netdev_dpdk_get_port_id(netdev));
> }
> } else {
> -OVS_NOT_REACHED();
> +VLOG_DBG_RL(&rl, "vport type '%s' is not supported",
> +netdev_get_type(vport));
> +return -1;
> }
>
> return 0;
> ---
>
> That looks good to me. So, I guess, Harsha, we're waiting for
> your review/tests here.
Thanks Ilya and Eli, looks good to me; I've also tested it and it works fine.
-Harsha
>
> >
> > Travis:
> > v1: https://travis-ci.org/github/elibritstein/OVS/builds/756418552
> > v2: https://travis-ci.org/github/elibritstein/OVS/builds/758382963
> > v3: https://travis-ci.org/github/elibritstein/OVS/builds/761089087
> > v4: https://travis-ci.org/github/elibritstein/OVS/builds/763146966
> > v5: https://travis-ci.org/github/elibritstein/OVS/builds/765271879
> > v6: https://travis-ci.org/github/elibritstein/OVS/builds/765816800
> > v7: Have a problem to run
>
> Yes, this thing is non-functional. Even travis-ci.com doesn't work
> for me for unknown reason (I do have compute credits).
>
> Best regards, Ilya Maximets.
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH V7 00/13] Netdev vxlan-decap offload
On 6/23/21 5:52 PM, Eli Britstein wrote:
> VXLAN decap in OVS-DPDK configuration consists of two flows:
> F1: in_port(ens1f0),eth(),ipv4(),udp(), actions:tnl_pop(vxlan_sys_4789)
> F2: tunnel(),in_port(vxlan_sys_4789),eth(),ipv4(), actions:ens1f0_0
>
> F1 is a classification flow. It has outer headers matches and it
> classifies the packet as a VXLAN packet, and using tnl_pop action the
> packet continues processing in F2.
> F2 is a flow that has matches on tunnel metadata as well as on the inner
> packet headers (as any other flow).
>
> In order to fully offload VXLAN decap path, both F1 and F2 should be
> offloaded. As there are more than one flow in HW, it is possible that
> F1 is done by HW but F2 is not. Packet is received by SW, and should be
> processed starting from F2 as F1 was already done by HW.
> Rte_flows are applicable only on physical port IDs. Keeping the original
> physical in port on which the packet is received on enables applying
> vport flows (e.g. F2) on that physical port.
>
> This patch-set makes use of [1] introduced in DPDK 20.11, that adds API
> for tunnel offloads.
>
> Note that MLX5 PMD has a bug that the tnl_pop private actions must be
> first. In OVS it is not.
> Fixing this issue is scheduled for 21.05 (and stable 20.11.2).
> Meanwhile, tests were done with a workaround for it [2].
>
> v2-v1:
> - Tracking original in_port, and applying vport on that physical port instead
> of all PFs.
> v3-v2:
> - Traversing ports using a new API instead of flow_dump.
> - Refactor packet state recover logic, with bug fix for error pop_header.
> - One ref count for netdev in non-tunnel case.
> - Rename variables, comments, rebase.
> v4-v3:
> - Extract orig_in_port from physdev for flow modify.
> - Miss handling fixes.
> v5-v4:
> - Drop refactor offload rule creation commit.
> - Comment about setting in_port in restore.
> - Refactor vports flow offload commit.
> v6-v5:
> - Fixed duplicate netdev ref bug.
> v7-v6:
> - Adopting Ilya's diff, with a minor fix in set_error stub.
> - Fixed abort (remove OVS_NOT_REACHED()) with tunnels other than vxlan
> ("netdev-offload-dpdk: Support tunnel pop action.").
Thanks! I see the only difference (beside the set_error fix) with what
I have locally is following:
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 363f32f71..6bd5b6c9f 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -835,7 +835,9 @@ vport_to_rte_tunnel(struct netdev *vport,
netdev_dpdk_get_port_id(netdev));
}
} else {
-OVS_NOT_REACHED();
+VLOG_DBG_RL(&rl, "vport type '%s' is not supported",
+netdev_get_type(vport));
+return -1;
}
return 0;
---
That looks good to me. So, I guess, Harsha, we're waiting for
your review/tests here.
>
> Travis:
> v1: https://travis-ci.org/github/elibritstein/OVS/builds/756418552
> v2: https://travis-ci.org/github/elibritstein/OVS/builds/758382963
> v3: https://travis-ci.org/github/elibritstein/OVS/builds/761089087
> v4: https://travis-ci.org/github/elibritstein/OVS/builds/763146966
> v5: https://travis-ci.org/github/elibritstein/OVS/builds/765271879
> v6: https://travis-ci.org/github/elibritstein/OVS/builds/765816800
> v7: Have a problem to run
Yes, this thing is non-functional. Even travis-ci.com doesn't work
for me for unknown reason (I do have compute credits).
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
