Re: [ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount
Done.
On Mon, Sep 23, 2019 at 02:26:11PM -0700, Yifeng Sun wrote:
> Hi Ben,
>
> Could you please backport this patch to 2.12? Thanks.
> Yifeng
>
> On Thu, Aug 29, 2019 at 8:39 AM Yifeng Sun wrote:
> >
> > Thanks Yi-Hung for the explanation.
> >
> > On Wed, Aug 28, 2019 at 4:49 PM Yi-Hung Wei wrote:
> > >
> > > On Wed, Aug 28, 2019 at 4:07 PM Ben Pfaff wrote:
> > > >
> > > > On Wed, Aug 07, 2019 at 03:25:33PM -0700, Yifeng Sun wrote:
> > > > > This patch backports several critical bug fixes related to
> > > > > locking and data consistency in nf_conncount code.
> > > > >
> > > > > This backport is based on the following upstream net-next upstream
> > > > > commits.
> > > > > a007232 ("netfilter: nf_conncount: fix argument order to
> > > > > find_next_bit")
> > > > > c80f10b ("netfilter: nf_conncount: speculative garbage collection on
> > > > > empty lists")
> > > > > 2f971a8 ("netfilter: nf_conncount: move all list iterations under
> > > > > spinlock")
> > > > > df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
> > > > > e8cfb37 ("netfilter: nf_conncount: restart search when nodes have
> > > > > been erased")
> > > > > f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
> > > > > 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is
> > > > > negative")
> > > > > c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with
> > > > > CONNCOUNT_SLOTS")
> > > > > d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of
> > > > > rb_link_node()")
> > > > > 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check
> > > > > routine")
> > > > > 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of
> > > > > list.")
> > > > > 31568ec ("netfilter: nf_conncount: fix list_del corruption in
> > > > > conn_free")
> > > > > fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of
> > > > > spin_lock")
> > > > >
> > > > > This patch adds additional compat code so that it can build on
> > > > > all supported kernel versions.
> > > >
> > > > I think that our most common approach is to use one OVS commit to
> > > > backport one Linux kernel commit. This commit combines many Linux
> > > > kernel commits. Is that an intentional change in this case?
> > >
> > > Hi Ben,
> > >
> > > Yes, we are intended to pull in all of the bug fixes in this case.
> > > The rationale is as following.
> > >
> > > For the commits in ovs kernel module, we usually backport one upstream
> > > net-next commit to one OVS commit. We need this fine granularity
> > > backports because a single OVS kernel module changes can affect OVS
> > > behavior. For the other type of kernel backports (mainly in
> > > ./datapath/linux/compat/ ), we try to backport the required missing
> > > features for ovs kernel module in the older kernel. The goal is to
> > > keep the older kernel in sync with the newer kernel on the required
> > > features, and we may not need much detailed information per upstream
> > > patch. In this case, it would be easier to pull in multiple patches
> > > at once.
> > >
> > > Some existing examples are,
> > > c387d8177f20 ("compat: Add ipv6 GRE and IPV6 Tunneling")
> > > 744964326f6c ("datapath: compat: Backports nf_conncount")
> > >
> > > Thanks,
> > >
> > > -Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount
Hi Ben,
Could you please backport this patch to 2.12? Thanks.
Yifeng
On Thu, Aug 29, 2019 at 8:39 AM Yifeng Sun wrote:
>
> Thanks Yi-Hung for the explanation.
>
> On Wed, Aug 28, 2019 at 4:49 PM Yi-Hung Wei wrote:
> >
> > On Wed, Aug 28, 2019 at 4:07 PM Ben Pfaff wrote:
> > >
> > > On Wed, Aug 07, 2019 at 03:25:33PM -0700, Yifeng Sun wrote:
> > > > This patch backports several critical bug fixes related to
> > > > locking and data consistency in nf_conncount code.
> > > >
> > > > This backport is based on the following upstream net-next upstream
> > > > commits.
> > > > a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit")
> > > > c80f10b ("netfilter: nf_conncount: speculative garbage collection on
> > > > empty lists")
> > > > 2f971a8 ("netfilter: nf_conncount: move all list iterations under
> > > > spinlock")
> > > > df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
> > > > e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been
> > > > erased")
> > > > f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
> > > > 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is
> > > > negative")
> > > > c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with
> > > > CONNCOUNT_SLOTS")
> > > > d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of
> > > > rb_link_node()")
> > > > 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check
> > > > routine")
> > > > 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of
> > > > list.")
> > > > 31568ec ("netfilter: nf_conncount: fix list_del corruption in
> > > > conn_free")
> > > > fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of
> > > > spin_lock")
> > > >
> > > > This patch adds additional compat code so that it can build on
> > > > all supported kernel versions.
> > >
> > > I think that our most common approach is to use one OVS commit to
> > > backport one Linux kernel commit. This commit combines many Linux
> > > kernel commits. Is that an intentional change in this case?
> >
> > Hi Ben,
> >
> > Yes, we are intended to pull in all of the bug fixes in this case.
> > The rationale is as following.
> >
> > For the commits in ovs kernel module, we usually backport one upstream
> > net-next commit to one OVS commit. We need this fine granularity
> > backports because a single OVS kernel module changes can affect OVS
> > behavior. For the other type of kernel backports (mainly in
> > ./datapath/linux/compat/ ), we try to backport the required missing
> > features for ovs kernel module in the older kernel. The goal is to
> > keep the older kernel in sync with the newer kernel on the required
> > features, and we may not need much detailed information per upstream
> > patch. In this case, it would be easier to pull in multiple patches
> > at once.
> >
> > Some existing examples are,
> > c387d8177f20 ("compat: Add ipv6 GRE and IPV6 Tunneling")
> > 744964326f6c ("datapath: compat: Backports nf_conncount")
> >
> > Thanks,
> >
> > -Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount
Thanks Yi-Hung for the explanation.
On Wed, Aug 28, 2019 at 4:49 PM Yi-Hung Wei wrote:
>
> On Wed, Aug 28, 2019 at 4:07 PM Ben Pfaff wrote:
> >
> > On Wed, Aug 07, 2019 at 03:25:33PM -0700, Yifeng Sun wrote:
> > > This patch backports several critical bug fixes related to
> > > locking and data consistency in nf_conncount code.
> > >
> > > This backport is based on the following upstream net-next upstream
> > > commits.
> > > a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit")
> > > c80f10b ("netfilter: nf_conncount: speculative garbage collection on
> > > empty lists")
> > > 2f971a8 ("netfilter: nf_conncount: move all list iterations under
> > > spinlock")
> > > df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
> > > e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been
> > > erased")
> > > f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
> > > 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is
> > > negative")
> > > c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with
> > > CONNCOUNT_SLOTS")
> > > d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of
> > > rb_link_node()")
> > > 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
> > > 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of
> > > list.")
> > > 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> > > fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
> > >
> > > This patch adds additional compat code so that it can build on
> > > all supported kernel versions.
> >
> > I think that our most common approach is to use one OVS commit to
> > backport one Linux kernel commit. This commit combines many Linux
> > kernel commits. Is that an intentional change in this case?
>
> Hi Ben,
>
> Yes, we are intended to pull in all of the bug fixes in this case.
> The rationale is as following.
>
> For the commits in ovs kernel module, we usually backport one upstream
> net-next commit to one OVS commit. We need this fine granularity
> backports because a single OVS kernel module changes can affect OVS
> behavior. For the other type of kernel backports (mainly in
> ./datapath/linux/compat/ ), we try to backport the required missing
> features for ovs kernel module in the older kernel. The goal is to
> keep the older kernel in sync with the newer kernel on the required
> features, and we may not need much detailed information per upstream
> patch. In this case, it would be easier to pull in multiple patches
> at once.
>
> Some existing examples are,
> c387d8177f20 ("compat: Add ipv6 GRE and IPV6 Tunneling")
> 744964326f6c ("datapath: compat: Backports nf_conncount")
>
> Thanks,
>
> -Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount
On Wed, Aug 28, 2019 at 04:48:52PM -0700, Yi-Hung Wei wrote:
> On Wed, Aug 28, 2019 at 4:07 PM Ben Pfaff wrote:
> >
> > On Wed, Aug 07, 2019 at 03:25:33PM -0700, Yifeng Sun wrote:
> > > This patch backports several critical bug fixes related to
> > > locking and data consistency in nf_conncount code.
> > >
> > > This backport is based on the following upstream net-next upstream
> > > commits.
> > > a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit")
> > > c80f10b ("netfilter: nf_conncount: speculative garbage collection on
> > > empty lists")
> > > 2f971a8 ("netfilter: nf_conncount: move all list iterations under
> > > spinlock")
> > > df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
> > > e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been
> > > erased")
> > > f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
> > > 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is
> > > negative")
> > > c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with
> > > CONNCOUNT_SLOTS")
> > > d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of
> > > rb_link_node()")
> > > 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
> > > 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of
> > > list.")
> > > 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> > > fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
> > >
> > > This patch adds additional compat code so that it can build on
> > > all supported kernel versions.
> >
> > I think that our most common approach is to use one OVS commit to
> > backport one Linux kernel commit. This commit combines many Linux
> > kernel commits. Is that an intentional change in this case?
>
> Hi Ben,
>
> Yes, we are intended to pull in all of the bug fixes in this case.
> The rationale is as following.
>
> For the commits in ovs kernel module, we usually backport one upstream
> net-next commit to one OVS commit. We need this fine granularity
> backports because a single OVS kernel module changes can affect OVS
> behavior. For the other type of kernel backports (mainly in
> ./datapath/linux/compat/ ), we try to backport the required missing
> features for ovs kernel module in the older kernel. The goal is to
> keep the older kernel in sync with the newer kernel on the required
> features, and we may not need much detailed information per upstream
> patch. In this case, it would be easier to pull in multiple patches
> at once.
>
> Some existing examples are,
> c387d8177f20 ("compat: Add ipv6 GRE and IPV6 Tunneling")
> 744964326f6c ("datapath: compat: Backports nf_conncount")
Thanks a lot.
I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount
On Wed, Aug 28, 2019 at 4:07 PM Ben Pfaff wrote:
>
> On Wed, Aug 07, 2019 at 03:25:33PM -0700, Yifeng Sun wrote:
> > This patch backports several critical bug fixes related to
> > locking and data consistency in nf_conncount code.
> >
> > This backport is based on the following upstream net-next upstream commits.
> > a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit")
> > c80f10b ("netfilter: nf_conncount: speculative garbage collection on empty
> > lists")
> > 2f971a8 ("netfilter: nf_conncount: move all list iterations under spinlock")
> > df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
> > e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been
> > erased")
> > f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
> > 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is
> > negative")
> > c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with
> > CONNCOUNT_SLOTS")
> > d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of
> > rb_link_node()")
> > 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
> > 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of list.")
> > 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> > fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
> >
> > This patch adds additional compat code so that it can build on
> > all supported kernel versions.
>
> I think that our most common approach is to use one OVS commit to
> backport one Linux kernel commit. This commit combines many Linux
> kernel commits. Is that an intentional change in this case?
Hi Ben,
Yes, we are intended to pull in all of the bug fixes in this case.
The rationale is as following.
For the commits in ovs kernel module, we usually backport one upstream
net-next commit to one OVS commit. We need this fine granularity
backports because a single OVS kernel module changes can affect OVS
behavior. For the other type of kernel backports (mainly in
./datapath/linux/compat/ ), we try to backport the required missing
features for ovs kernel module in the older kernel. The goal is to
keep the older kernel in sync with the newer kernel on the required
features, and we may not need much detailed information per upstream
patch. In this case, it would be easier to pull in multiple patches
at once.
Some existing examples are,
c387d8177f20 ("compat: Add ipv6 GRE and IPV6 Tunneling")
744964326f6c ("datapath: compat: Backports nf_conncount")
Thanks,
-Yi-Hung
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount
On Wed, Aug 07, 2019 at 03:25:33PM -0700, Yifeng Sun wrote:
> This patch backports several critical bug fixes related to
> locking and data consistency in nf_conncount code.
>
> This backport is based on the following upstream net-next upstream commits.
> a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit")
> c80f10b ("netfilter: nf_conncount: speculative garbage collection on empty
> lists")
> 2f971a8 ("netfilter: nf_conncount: move all list iterations under spinlock")
> df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
> e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been
> erased")
> f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
> 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is negative")
> c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with
> CONNCOUNT_SLOTS")
> d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of
> rb_link_node()")
> 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
> 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of list.")
> 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
>
> This patch adds additional compat code so that it can build on
> all supported kernel versions.
I think that our most common approach is to use one OVS commit to
backport one Linux kernel commit. This commit combines many Linux
kernel commits. Is that an intentional change in this case?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] datapath: compat: Backports bugfixes for nf_conncount
On Wed, Aug 7, 2019 at 3:25 PM Yifeng Sun wrote:
>
> This patch backports several critical bug fixes related to
> locking and data consistency in nf_conncount code.
>
> This backport is based on the following upstream net-next upstream commits.
> a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit")
> c80f10b ("netfilter: nf_conncount: speculative garbage collection on empty
> lists")
> 2f971a8 ("netfilter: nf_conncount: move all list iterations under spinlock")
> df4a902 ("netfilter: nf_conncount: merge lookup and add functions")
> e8cfb37 ("netfilter: nf_conncount: restart search when nodes have been
> erased")
> f7fcc98 ("netfilter: nf_conncount: split gc in two phases")
> 4cd273b ("netfilter: nf_conncount: don't skip eviction when age is negative")
> c78e781 ("netfilter: nf_conncount: replace CONNCOUNT_LOCK_SLOTS with
> CONNCOUNT_SLOTS")
> d4e7df1 ("netfilter: nf_conncount: use rb_link_node_rcu() instead of
> rb_link_node()")
> 53ca0f2 ("netfilter: nf_conncount: remove wrong condition check routine")
> 3c5cdb1 ("netfilter: nf_conncount: fix unexpected permanent node of list.")
> 31568ec ("netfilter: nf_conncount: fix list_del corruption in conn_free")
> fd3e71a ("netfilter: nf_conncount: use spin_lock_bh instead of spin_lock")
>
> This patch adds additional compat code so that it can build on
> all supported kernel versions.
>
> In addition, this patch helps OVS datapath to always choose bug-fixed
> nf_conncount code. If kernel already has these fixes, then kernel's
> nf_conncount is being used. Otherwise, OVS falls back to use compat
> nf_conncount functions.
>
> Travis tests are at
> https://travis-ci.org/yifsun/ovs-travis/builds/569056850
> On latest RHEL kernel, 'make check-kmod' runs good.
>
> VMware-BZ: #2396471
>
> Signed-off-by: Yifeng Sun
> ---
Thanks Yifeng for the fix. This patch looks good to me.
Acked-by: Yi-Hung Wei
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
