[ovs-dev] 回复: 回复: one issue in vxlan functionality of the kernel-datapath type of ovs
diff mbox series

Message ID HK0PR03MB4049826675329F17FACE4A78B21B0@HK0PR03MB4049.apcprd03.prod.outlook.com
State New
Headers show
Series
  • [ovs-dev] 回复: 回复: one issue in vxlan functionality of the kernel-datapath type of ovs
Related show

Commit Message

pei Jikui June 2, 2019, 1:52 a.m. UTC
Attach the diff which I have verified locally.
Thanks.

[root@localhost ovs]# git diff

Comments

Ben Pfaff June 5, 2019, 7:58 p.m. UTC | #1
Thanks for working on the problem.

I agree that this will solve this particular problem.  At the same time,
it will break OVS in every situation where a flow's actions need to
change from "drop" to anything else.  We can't accept that regression,
which would be a correctness issue, not just a performance issue.

On Sun, Jun 02, 2019 at 01:52:56AM +0000, pei Jikui wrote:
> Attach the diff which I have verified locally.
> Thanks.
> 
> [root@localhost ovs]# git diff
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index dc30824..c5a7de6 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -2678,9 +2678,12 @@ revalidate(struct revalidator *revalidator)
>              }
>              if (kill_them_all || (used && used < now - max_idle)) {
>                  result = UKEY_DELETE;
> -            } else {
> +            /*Only validate the ukey if the flow's action is not drop.Since for the drop flows, there might be not validated.*/
> +            } else if (f->actions_len > 0) {
>                  result = revalidate_ukey(udpif, ukey, &f->stats, &odp_actions,
>                                           reval_seq, &recircs);
> +            } else {
> +                result = UKEY_KEEP;
>              }
>              ukey->dump_seq = dump_seq;
> 
> 
> 
> ________________________________
> 发件人: ovs-dev-bounces@openvswitch.org <ovs-dev-bounces@openvswitch.org> 代表 pei Jikui <jkpei@hotmail.com>
> 发送时间: 2019年6月2日 9:42
> 收件人: Ben Pfaff
> 抄送: ovs-dev@openvswitch.org; ovs-discuss@openvswitch.org
> 主题: [ovs-dev] 回复: one issue in vxlan functionality of the kernel-datapath type of ovs
> 
> hi,
> 
> I looked into this further and found the root cause.
> 
> 1) for the unwanted packets(no vxlan tunnel configured for this case), the receiver will install the drop-action flow to the datapath.
> 2) when the revalidator thread is trying to age-out/clean-up the flows installed in step 1), they are considered as invalidated by revalidator thread since the revalidator thread could not find the xport for them. Hence the revalidator thread delete the them very soon.
> 3) that is causing all the upcoming packets are sending to userspace again.   And repeat the steps that  a) send to userspace, b) install drop-action flow table c) the revalidator thread delete them.
> 4) A proposed fix is if the flow's action is drop, we only bank on the time age-out mechanism to clean them up and don't enforce the revalidate_ukey action for them.
> 
> Thanks.
> 
> Jikui
> 
> ________________________________
> 发件人: pei Jikui <jkpei@hotmail.com>
> 发送时间: 2019年5月25日 7:31
> 收件人: Ben Pfaff
> 抄送: ovs-dev@openvswitch.org; ovs-discuss@openvswitch.org
> 主题: Re: [ovs-dev] one issue in vxlan functionality of the kernel-datapath type of ovs
> 
> In my test bed there is no any dropping flow entry generated in the datapath, which causes all the unwanted vxlan packets will go to the slow path.  That’s a little time-consuming for this case.
> 
> 
> 
> 发送自 Outlook<http://aka.ms/weboutlook>
> 
> ________________________________
> 发件人: Ben Pfaff <blp@ovn.org>
> 发送时间: 2019年5月25日 2:36
> 收件人: pei Jikui
> 抄送: ovs-dev@openvswitch.org; ovs-discuss@openvswitch.org
> 主题: Re: [ovs-dev] one issue in vxlan functionality of the kernel-datapath type of ovs
> 
> On Sat, May 18, 2019 at 10:23:50AM +0000, pei Jikui wrote:
> > I found one issue in the vxlan functionality in kernel-datapath type of ovs which could be potentially optimize?
> >
> >
> > For example, there is a machine (A) running ovs with one configured one vxlan interface with tunnel value 123,  then all the vxlan traffics destinate to this machine(A) will be dealt with the ovs.
> >
> >
> > Although the ovs in machine A only configured with one vxlan tunnel (123), all the vxlan traffics regardless the tunnel value is 123 or not, will be delivered to the vswitchd to do the slow path if there is no flow tables found in the datapath.
> >
> > This is due to the vxlan configuration such as the configured vxlan tunnel valuse does not exist in the datapath. (There is only one vxlan tunnel with vni value 0 exist in the datapath’s vxlan configuration).
> 
> It's true, but does it matter?  It would be unusual for a host to
> receive much VXLAN traffic that it is only going to drop.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
pei Jikui June 10, 2019, 1:19 a.m. UTC | #2
Ben,

Thanks for your review.

1) Based on my understanding, there are two kinds of cases to age out the datapath flow.
    a) Timer.
    b) revalidate. The flow will be deleted if there are some invalidated conditions such as there is no output interface for this flow.

Even if the revalidate does not work, the timer mechanism will eventually delete the flows.


2) Would you please help to elaborate what the impaction of this change imposes to the case where the actions are changed from "drop" to anything else?  I couldn't understand this well.


Thanks in advance.
Ben Pfaff June 10, 2019, 4:17 p.m. UTC | #3
On Mon, Jun 10, 2019 at 01:19:33AM +0000, pei Jikui wrote:
> 1) Based on my understanding, there are two kinds of cases to age out the datapath flow.
>     a) Timer.
>     b) revalidate. The flow will be deleted if there are some invalidated conditions such as there is no output interface for this flow.
> 
> Even if the revalidate does not work, the timer mechanism will eventually delete the flows.

The timer mechanism is only used if a flow becomes idle, which can take
an indefinite time.

> 2) Would you please help to elaborate what the impaction of this change imposes to the case where the actions are changed from "drop" to anything else?  I couldn't understand this well.

If the controller changes the OpenFlow flow table, then revalidation
might change datapath flow actions arbitrarily.  Some of those changes
can be from "drop" to something else.

> Thanks for working on the problem.
> 
> I agree that this will solve this particular problem.  At the same time,
> it will break OVS in every situation where a flow's actions need to
> change from "drop" to anything else.  We can't accept that regression,
> which would be a correctness issue, not just a performance issue.
> 
> On Sun, Jun 02, 2019 at 01:52:56AM +0000, pei Jikui wrote:
> > Attach the diff which I have verified locally.
> > Thanks.
> >
> > [root@localhost ovs]# git diff
> > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> > index dc30824..c5a7de6 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -2678,9 +2678,12 @@ revalidate(struct revalidator *revalidator)
> >              }
> >              if (kill_them_all || (used && used < now - max_idle)) {
> >                  result = UKEY_DELETE;
> > -            } else {
> > +            /*Only validate the ukey if the flow's action is not drop.Since for the drop flows, there might be not validated.*/
> > +            } else if (f->actions_len > 0) {
> >                  result = revalidate_ukey(udpif, ukey, &f->stats, &odp_actions,
> >                                           reval_seq, &recircs);
> > +            } else {
> > +                result = UKEY_KEEP;
> >              }
> >              ukey->dump_seq = dump_seq;
> >
> >
> >
> > ________________________________
> > 发件人: ovs-dev-bounces@openvswitch.org <ovs-dev-bounces@openvswitch.org> 代表 pei Jikui <jkpei@hotmail.com>
> > 发送时间: 2019年6月2日 9:42
> > 收件人: Ben Pfaff
> > 抄送: ovs-dev@openvswitch.org; ovs-discuss@openvswitch.org
> > 主题: [ovs-dev] 回复: one issue in vxlan functionality of the kernel-datapath type of ovs
> >
> > hi,
> >
> > I looked into this further and found the root cause.
> >
> > 1) for the unwanted packets(no vxlan tunnel configured for this case), the receiver will install the drop-action flow to the datapath.
> > 2) when the revalidator thread is trying to age-out/clean-up the flows installed in step 1), they are considered as invalidated by revalidator thread since the revalidator thread could not find the xport for them. Hence the revalidator thread delete the them very soon.
> > 3) that is causing all the upcoming packets are sending to userspace again.   And repeat the steps that  a) send to userspace, b) install drop-action flow table c) the revalidator thread delete them.
> > 4) A proposed fix is if the flow's action is drop, we only bank on the time age-out mechanism to clean them up and don't enforce the revalidate_ukey action for them.
> >
> > Thanks.
> >
> > Jikui
> >
> > ________________________________
> > 发件人: pei Jikui <jkpei@hotmail.com>
> > 发送时间: 2019年5月25日 7:31
> > 收件人: Ben Pfaff
> > 抄送: ovs-dev@openvswitch.org; ovs-discuss@openvswitch.org
> > 主题: Re: [ovs-dev] one issue in vxlan functionality of the kernel-datapath type of ovs
> >
> > In my test bed there is no any dropping flow entry generated in the datapath, which causes all the unwanted vxlan packets will go to the slow path.  That’s a little time-consuming for this case.
> >
> >
> >
> > 发送自 Outlook<http://aka.ms/weboutlook>
> >
> > ________________________________
> > 发件人: Ben Pfaff <blp@ovn.org>
> > 发送时间: 2019年5月25日 2:36
> > 收件人: pei Jikui
> > 抄送: ovs-dev@openvswitch.org; ovs-discuss@openvswitch.org
> > 主题: Re: [ovs-dev] one issue in vxlan functionality of the kernel-datapath type of ovs
> >
> > On Sat, May 18, 2019 at 10:23:50AM +0000, pei Jikui wrote:
> > > I found one issue in the vxlan functionality in kernel-datapath type of ovs which could be potentially optimize?
> > >
> > >
> > > For example, there is a machine (A) running ovs with one configured one vxlan interface with tunnel value 123,  then all the vxlan traffics destinate to this machine(A) will be dealt with the ovs.
> > >
> > >
> > > Although the ovs in machine A only configured with one vxlan tunnel (123), all the vxlan traffics regardless the tunnel value is 123 or not, will be delivered to the vswitchd to do the slow path if there is no flow tables found in the datapath.
> > >
> > > This is due to the vxlan configuration such as the configured vxlan tunnel valuse does not exist in the datapath. (There is only one vxlan tunnel with vni value 0 exist in the datapath’s vxlan configuration).
> >
> > It's true, but does it matter?  It would be unusual for a host to
> > receive much VXLAN traffic that it is only going to drop.
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Patch
diff mbox series

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index dc30824..c5a7de6 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2678,9 +2678,12 @@  revalidate(struct revalidator *revalidator)
             }
             if (kill_them_all || (used && used < now - max_idle)) {
                 result = UKEY_DELETE;
-            } else {
+            /*Only validate the ukey if the flow's action is not drop.Since for the drop flows, there might be not validated.*/
+            } else if (f->actions_len > 0) {
                 result = revalidate_ukey(udpif, ukey, &f->stats, &odp_actions,
                                          reval_seq, &recircs);
+            } else {
+                result = UKEY_KEEP;
             }
             ukey->dump_seq = dump_seq;