diff mbox series

[ovs-dev,ovs] dpif-netdev: Delete the flow limit in dpif-netdev

Message ID 1584309363-15048-1-git-send-email-xiangxia.m.yue@gmail.com
State Accepted
Delegated to: Ilya Maximets
Headers show
Series [ovs-dev,ovs] dpif-netdev: Delete the flow limit in dpif-netdev | expand

Commit Message

Tonghao Zhang March 15, 2020, 9:56 p.m. UTC 
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

The max number of flow in dpif-netdev is MAX_FLOWS,
the previous patch[1] try to install more flows,
and introduce the subcommand "dpif-netdev/pmd-set-max-flow"
to change the max flow limitation. But 'other_config:flow-limit'
option can limit that too.

As Ilya Maximets say [2]:
| The MAX_FLOWS constant was there from the introduction of dpif-netdev,
| however, later new flow-limit mechanism was implemented that
| controls number of datapath flows in a dynamic way on ofproto level.
|
| So, maybe we can just remove the limit and fully rely on ofproto
| to decide what flow limit we need?  There are no limitations for
| flow table size in dpif-netdev beside the artificial one.
| 'other_config:flow-limit' seems suitable to control this.

[1] - https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366636.html
[2] - https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366677.html

Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/dpif-netdev.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

Comments

Tonghao Zhang June 16, 2020, 12:15 p.m. UTC | #1 
On Tue, Jun 16, 2020 at 8:12 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
>
> On Mon, Mar 16, 2020 at 7:17 PM <xiangxia.m.yue@gmail.com> wrote:
> >
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > The max number of flow in dpif-netdev is MAX_FLOWS,
> > the previous patch[1] try to install more flows,
> > and introduce the subcommand "dpif-netdev/pmd-set-max-flow"
> > to change the max flow limitation. But 'other_config:flow-limit'
> > option can limit that too.
> >
> > As Ilya Maximets say [2]:
> > | The MAX_FLOWS constant was there from the introduction of dpif-netdev,
> > | however, later new flow-limit mechanism was implemented that
> > | controls number of datapath flows in a dynamic way on ofproto level.
> > |
> > | So, maybe we can just remove the limit and fully rely on ofproto
> > | to decide what flow limit we need?  There are no limitations for
> > | flow table size in dpif-netdev beside the artificial one.
> > | 'other_config:flow-limit' seems suitable to control this.
> >
> > [1] - https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366636.html
> > [2] - https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366677.html
> >
> > Suggested-by: Ilya Maximets <i.maximets@ovn.org>
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> any progress ?
cc dev@openvswitch.org
any progress ? Do we have plan to review it or pick it up.
> > ---
> >  lib/dpif-netdev.c | 12 ++----------
> >  1 file changed, 2 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index a1c45f6..eb0e2da 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -96,9 +96,6 @@ DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
> >  /* Use instant packet send by default. */
> >  #define DEFAULT_TX_FLUSH_INTERVAL 0
> >
> > -/* Configuration parameters. */
> > -enum { MAX_FLOWS = 65536 };     /* Maximum number of flows in flow table. */
> > -
> >  /* Maximum number of meters in the table. */
> >  #define METER_ENTRY_MAX (1 << 19)
> >  /* Maximum number of bands / meter. */
> > @@ -3576,13 +3573,8 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
> >      netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
> >      if (!netdev_flow) {
> >          if (put->flags & DPIF_FP_CREATE) {
> > -            if (cmap_count(&pmd->flow_table) < MAX_FLOWS) {
> > -                dp_netdev_flow_add(pmd, match, ufid, put->actions,
> > -                                   put->actions_len);
> > -                error = 0;
> > -            } else {
> > -                error = EFBIG;
> > -            }
> > +            dp_netdev_flow_add(pmd, match, ufid, put->actions,
> > +                               put->actions_len);
> >          } else {
> >              error = ENOENT;
> >          }
> > --
> > 1.8.3.1
> >
>
>
> --
> Best regards, Tonghao
Ilya Maximets July 7, 2020, 12:09 a.m. UTC | #2 
On 6/16/20 2:15 PM, Tonghao Zhang wrote:
> On Tue, Jun 16, 2020 at 8:12 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
>>
>> On Mon, Mar 16, 2020 at 7:17 PM <xiangxia.m.yue@gmail.com> wrote:
>>>
>>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>>
>>> The max number of flow in dpif-netdev is MAX_FLOWS,
>>> the previous patch[1] try to install more flows,
>>> and introduce the subcommand "dpif-netdev/pmd-set-max-flow"
>>> to change the max flow limitation. But 'other_config:flow-limit'
>>> option can limit that too.
>>>
>>> As Ilya Maximets say [2]:
>>> | The MAX_FLOWS constant was there from the introduction of dpif-netdev,
>>> | however, later new flow-limit mechanism was implemented that
>>> | controls number of datapath flows in a dynamic way on ofproto level.
>>> |
>>> | So, maybe we can just remove the limit and fully rely on ofproto
>>> | to decide what flow limit we need?  There are no limitations for
>>> | flow table size in dpif-netdev beside the artificial one.
>>> | 'other_config:flow-limit' seems suitable to control this.
>>>
>>> [1] - https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366636.html
>>> [2] - https://mail.openvswitch.org/pipermail/ovs-dev/2020-January/366677.html
>>>
>>> Suggested-by: Ilya Maximets <i.maximets@ovn.org>
>>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> any progress ?
> cc dev@openvswitch.org
> any progress ? Do we have plan to review it or pick it up.
>>> ---
>>>  lib/dpif-netdev.c | 12 ++----------
>>>  1 file changed, 2 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index a1c45f6..eb0e2da 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -96,9 +96,6 @@ DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
>>>  /* Use instant packet send by default. */
>>>  #define DEFAULT_TX_FLUSH_INTERVAL 0
>>>
>>> -/* Configuration parameters. */
>>> -enum { MAX_FLOWS = 65536 };     /* Maximum number of flows in flow table. */
>>> -
>>>  /* Maximum number of meters in the table. */
>>>  #define METER_ENTRY_MAX (1 << 19)
>>>  /* Maximum number of bands / meter. */
>>> @@ -3576,13 +3573,8 @@ flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
>>>      netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
>>>      if (!netdev_flow) {
>>>          if (put->flags & DPIF_FP_CREATE) {
>>> -            if (cmap_count(&pmd->flow_table) < MAX_FLOWS) {
>>> -                dp_netdev_flow_add(pmd, match, ufid, put->actions,
>>> -                                   put->actions_len);
>>> -                error = 0;
>>> -            } else {
>>> -                error = EFBIG;
>>> -            }
>>> +            dp_netdev_flow_add(pmd, match, ufid, put->actions,
>>> +                               put->actions_len);
>>>          } else {
>>>              error = ENOENT;
>>>          }
>>> --
>>> 1.8.3.1

Thanks!

Apparently, this patch is not based on a master branch.  Please, be sure
that you're formatting patches for the master branch next time.

For now I rebased it manually, added relevant NEWS entry and applied.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index a1c45f6..eb0e2da 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -96,9 +96,6 @@  DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
 /* Use instant packet send by default. */
 #define DEFAULT_TX_FLUSH_INTERVAL 0
 
-/* Configuration parameters. */
-enum { MAX_FLOWS = 65536 };     /* Maximum number of flows in flow table. */
-
 /* Maximum number of meters in the table. */
 #define METER_ENTRY_MAX (1 << 19)
 /* Maximum number of bands / meter. */
@@ -3576,13 +3573,8 @@  flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
     netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
     if (!netdev_flow) {
         if (put->flags & DPIF_FP_CREATE) {
-            if (cmap_count(&pmd->flow_table) < MAX_FLOWS) {
-                dp_netdev_flow_add(pmd, match, ufid, put->actions,
-                                   put->actions_len);
-                error = 0;
-            } else {
-                error = EFBIG;
-            }
+            dp_netdev_flow_add(pmd, match, ufid, put->actions,
+                               put->actions_len);
         } else {
             error = ENOENT;
         }