| Message ID | 1533033639-13009-1-git-send-email-ogerlitz@mellanox.com |
|---|---|
| Headers | show |
| Series | Enable set/match of tos/ttl for IP tunnels on TC data-path | expand |
On Tue, Jul 31, 2018 at 01:40:35PM +0300, Or Gerlitz wrote: > This series comes to address the case to set (encap) and match (decap) > also the tos and ttl fields of TC based IP tunnels. This happens e.g > due to inherit setup of tunnel port for tos or due to specific OF rule. > > The series is rebased over Jianbo's patches for QinQ [1] I verified that these build and pass the unit tests. I didn't review them otherwise.
On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz <ogerlitz@mellanox.com> wrote: > This series comes to address the case to set (encap) and match (decap) > also the tos and ttl fields of TC based IP tunnels. This happens e.g > due to inherit setup of tunnel port for tos or due to specific OF rule. > > The series is rebased over Jianbo's patches for QinQ [1] FWIW - note that v2 was actually rebased to the master where Jianbo's work is already applied
Thanks Or, Thanks Ben, On 1 August 2018 at 08:43, Or Gerlitz <gerlitz.or@gmail.com> wrote: > On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz <ogerlitz@mellanox.com> wrote: > > This series comes to address the case to set (encap) and match (decap) > > also the tos and ttl fields of TC based IP tunnels. This happens e.g > > due to inherit setup of tunnel port for tos or due to specific OF rule. > > > > The series is rebased over Jianbo's patches for QinQ [1] > > FWIW - note that v2 was actually rebased to the master where Jianbo's work > is already applied > I have also reviewed these patches, tested that travis-ci is happy with everything when applied on top of 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB databases."), which was the most recent travis-ci-clean commit in the master branch yesterday, and Netronome has performed some testing in the lab. Overall I am happy with these patches and plan to apply them later today after one final run through travis-ci after rebasing onto the current master branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2 3/3] ovn-northd: Propagate dynamic addresses to port group address sets."]).
On 1 August 2018 at 11:31, Simon Horman <simon.horman@netronome.com> wrote: > Thanks Or, Thanks Ben, > > On 1 August 2018 at 08:43, Or Gerlitz <gerlitz.or@gmail.com> wrote: > >> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz <ogerlitz@mellanox.com> >> wrote: >> > This series comes to address the case to set (encap) and match (decap) >> > also the tos and ttl fields of TC based IP tunnels. This happens e.g >> > due to inherit setup of tunnel port for tos or due to specific OF rule. >> > >> > The series is rebased over Jianbo's patches for QinQ [1] >> >> FWIW - note that v2 was actually rebased to the master where Jianbo's work >> is already applied >> > > I have also reviewed these patches, tested that travis-ci is happy with > everything when applied on top of > 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB > databases."), which was the most recent > travis-ci-clean commit in the master branch yesterday, and Netronome has > performed some testing in the lab. > > Overall I am happy with these patches and plan to apply them later today > after one final run through travis-ci after rebasing onto the current > master branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH > v2 3/3] ovn-northd: Propagate dynamic addresses to port group address > sets."]). > Thanks again Or, I have applied this series to master.
On Wed, Aug 1, 2018 at 2:07 PM, Simon Horman <simon.horman@netronome.com> wrote: > On 1 August 2018 at 11:31, Simon Horman <simon.horman@netronome.com> wrote: >> >> Thanks Or, Thanks Ben, >> >> On 1 August 2018 at 08:43, Or Gerlitz <gerlitz.or@gmail.com> wrote: >>> >>> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz <ogerlitz@mellanox.com> >>> wrote: >>> > This series comes to address the case to set (encap) and match (decap) >>> > also the tos and ttl fields of TC based IP tunnels. This happens e.g >>> > due to inherit setup of tunnel port for tos or due to specific OF rule. >>> > >>> > The series is rebased over Jianbo's patches for QinQ [1] >>> >>> FWIW - note that v2 was actually rebased to the master where Jianbo's >>> work >>> is already applied >> >> >> I have also reviewed these patches, tested that travis-ci is happy with >> everything when applied on top of >> 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB >> databases."), which was the most recent >> travis-ci-clean commit in the master branch yesterday, and Netronome has >> performed some testing in the lab. >> >> Overall I am happy with these patches and plan to apply them later today >> after one final run through travis-ci after rebasing onto the current master >> branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2 3/3] >> ovn-northd: Propagate dynamic addresses to port group address sets."]). > Thanks again Or, I have applied this series to master. Thank you. So how is the stable process @ ovs goes? is that documented, where? e.g b4496fc "lib/tc: Handle ttl for ipv6 too" is a bug fix, should/who I ask for stable inclusion?
Hi Or, On 1 August 2018 at 13:21, Or Gerlitz <gerlitz.or@gmail.com> wrote: > On Wed, Aug 1, 2018 at 2:07 PM, Simon Horman <simon.horman@netronome.com> > wrote: > > On 1 August 2018 at 11:31, Simon Horman <simon.horman@netronome.com> > wrote: > >> > >> Thanks Or, Thanks Ben, > >> > >> On 1 August 2018 at 08:43, Or Gerlitz <gerlitz.or@gmail.com> wrote: > >>> > >>> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz <ogerlitz@mellanox.com> > >>> wrote: > >>> > This series comes to address the case to set (encap) and match > (decap) > >>> > also the tos and ttl fields of TC based IP tunnels. This happens e.g > >>> > due to inherit setup of tunnel port for tos or due to specific OF > rule. > >>> > > >>> > The series is rebased over Jianbo's patches for QinQ [1] > >>> > >>> FWIW - note that v2 was actually rebased to the master where Jianbo's > >>> work > >>> is already applied > >> > >> > >> I have also reviewed these patches, tested that travis-ci is happy with > >> everything when applied on top of > >> 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB > >> databases."), which was the most recent > >> travis-ci-clean commit in the master branch yesterday, and Netronome has > >> performed some testing in the lab. > >> > >> Overall I am happy with these patches and plan to apply them later today > >> after one final run through travis-ci after rebasing onto the current > master > >> branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2 > 3/3] > >> ovn-northd: Propagate dynamic addresses to port group address sets."]). > > > Thanks again Or, I have applied this series to master. > > > Thank you. > > So how is the stable process @ ovs goes? is that documented, where? > e.g b4496fc "lib/tc: Handle ttl for ipv6 too" is a bug fix, should/who I > ask > for stable inclusion? > Hi Or, The usual procedure, as I understand, is to ask if the maintainer doesn't apply the fix to the desired stable branches. I'll take the above as a request to apply the patch to branch-2.10. Do you want it considered for any other stable branches?
On Wed, Aug 1, 2018 at 2:29 PM, Simon Horman <simon.horman@netronome.com> wrote: > Hi Or, > > On 1 August 2018 at 13:21, Or Gerlitz <gerlitz.or@gmail.com> wrote: >> >> On Wed, Aug 1, 2018 at 2:07 PM, Simon Horman <simon.horman@netronome.com> >> wrote: >> > On 1 August 2018 at 11:31, Simon Horman <simon.horman@netronome.com> >> > wrote: >> >> >> >> Thanks Or, Thanks Ben, >> >> >> >> On 1 August 2018 at 08:43, Or Gerlitz <gerlitz.or@gmail.com> wrote: >> >>> >> >>> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz <ogerlitz@mellanox.com> >> >>> wrote: >> >>> > This series comes to address the case to set (encap) and match >> >>> > (decap) >> >>> > also the tos and ttl fields of TC based IP tunnels. This happens e.g >> >>> > due to inherit setup of tunnel port for tos or due to specific OF >> >>> > rule. >> >>> > >> >>> > The series is rebased over Jianbo's patches for QinQ [1] >> >>> >> >>> FWIW - note that v2 was actually rebased to the master where Jianbo's >> >>> work >> >>> is already applied >> >> >> >> >> >> I have also reviewed these patches, tested that travis-ci is happy with >> >> everything when applied on top of >> >> 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB >> >> databases."), which was the most recent >> >> travis-ci-clean commit in the master branch yesterday, and Netronome >> >> has >> >> performed some testing in the lab. >> >> >> >> Overall I am happy with these patches and plan to apply them later >> >> today >> >> after one final run through travis-ci after rebasing onto the current >> >> master >> >> branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2 >> >> 3/3] >> >> ovn-northd: Propagate dynamic addresses to port group address sets."]). >> >> > Thanks again Or, I have applied this series to master. >> >> >> Thank you. >> >> So how is the stable process @ ovs goes? is that documented, where? >> e.g b4496fc "lib/tc: Handle ttl for ipv6 too" is a bug fix, should/who I >> ask >> for stable inclusion? > The usual procedure, as I understand, is to ask if the maintainer doesn't apply > the fix to the desired stable branches. I'll take the above as a request to > apply the patch to branch-2.10. > Do you want it considered for any other stable branches? Hi Simon, Yes, please do apply the ttl fix to 2.10 and if possible, to 2.9 as well since the bug was introduced there. Also, it would be good if dfa2ccd "lib/tc: Support matching on ip tos" would also go to 2.10. I realized that commit 8f283af "netdev-tc-offloads: Implement netdev flow put using tc interface" has blindly set the tos field @ the mask to zero (see mask->nw_tos = 0 in netdev_tc_flow_put) as if we offloaded that to the TC DP, but we didn't.. Or.
On Wed, Aug 01, 2018 at 04:28:03PM +0300, Or Gerlitz wrote: > On Wed, Aug 1, 2018 at 2:29 PM, Simon Horman <simon.horman@netronome.com> wrote: > > Hi Or, > > > > On 1 August 2018 at 13:21, Or Gerlitz <gerlitz.or@gmail.com> wrote: > >> > >> On Wed, Aug 1, 2018 at 2:07 PM, Simon Horman <simon.horman@netronome.com> > >> wrote: > >> > On 1 August 2018 at 11:31, Simon Horman <simon.horman@netronome.com> > >> > wrote: > >> >> > >> >> Thanks Or, Thanks Ben, > >> >> > >> >> On 1 August 2018 at 08:43, Or Gerlitz <gerlitz.or@gmail.com> wrote: > >> >>> > >> >>> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz <ogerlitz@mellanox.com> > >> >>> wrote: > >> >>> > This series comes to address the case to set (encap) and match > >> >>> > (decap) > >> >>> > also the tos and ttl fields of TC based IP tunnels. This happens e.g > >> >>> > due to inherit setup of tunnel port for tos or due to specific OF > >> >>> > rule. > >> >>> > > >> >>> > The series is rebased over Jianbo's patches for QinQ [1] > >> >>> > >> >>> FWIW - note that v2 was actually rebased to the master where Jianbo's > >> >>> work > >> >>> is already applied > >> >> > >> >> > >> >> I have also reviewed these patches, tested that travis-ci is happy with > >> >> everything when applied on top of > >> >> 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB > >> >> databases."), which was the most recent > >> >> travis-ci-clean commit in the master branch yesterday, and Netronome > >> >> has > >> >> performed some testing in the lab. > >> >> > >> >> Overall I am happy with these patches and plan to apply them later > >> >> today > >> >> after one final run through travis-ci after rebasing onto the current > >> >> master > >> >> branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2 > >> >> 3/3] > >> >> ovn-northd: Propagate dynamic addresses to port group address sets."]). > >> > >> > Thanks again Or, I have applied this series to master. > >> > >> > >> Thank you. > >> > >> So how is the stable process @ ovs goes? is that documented, where? > >> e.g b4496fc "lib/tc: Handle ttl for ipv6 too" is a bug fix, should/who I > >> ask > >> for stable inclusion? > > > The usual procedure, as I understand, is to ask if the maintainer doesn't apply > > the fix to the desired stable branches. I'll take the above as a request to > > apply the patch to branch-2.10. > > Do you want it considered for any other stable branches? > > Hi Simon, > > Yes, please do apply the ttl fix to 2.10 and if possible, to 2.9 as well since > the bug was introduced there. > > Also, it would be good if dfa2ccd "lib/tc: Support matching on ip tos" > would also go to 2.10. > I realized that commit 8f283af "netdev-tc-offloads: Implement netdev > flow put using tc interface" > has blindly set the tos field @ the mask to zero (see mask->nw_tos = 0 > in netdev_tc_flow_put) > as if we offloaded that to the TC DP, but we didn't.. Thanks, I backported and pushed: * to branch-2.9 - b4496fc "lib/tc: Handle ttl for ipv6 too" * to branch-2.10 - b4496fc "lib/tc: Handle ttl for ipv6 too" - dfa2ccd "lib/tc: Support matching on ip tos"