| Message ID | 1620232983-156509-1-git-send-email-danielj@nvidia.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,F:linux-bluefield] UBUNTU: SAUCE: net/sched: Increase support of reclassification to 10 | expand |
I hate to be nit picky, but the subject line reclassify loop limit doesn't match the value in the patch. What kind of performance impact will this incur ? Are unclassifiable packets common ? On 5/5/21 10:43 AM, Daniel Jurgens wrote: > From: Roi Dayan <roid@nvidia.com> > > BugLink: https://bugs.launchpad.net/bugs/1927257 > > To support packet reclassification of more than 4 chains > increase max_reclassify_loop from 4 to 15. > > Change-Id: Ie18c37fb0255a737dc8f1b2fcf10a1163266dde6 > Signed-off-by: Roi Dayan <roid@nvidia.com> > Reviewed-by: Oz Shlomo <ozsh@nvidia.com> > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> > --- > net/sched/cls_api.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c > index b6e0e15..9c9afc0 100644 > --- a/net/sched/cls_api.c > +++ b/net/sched/cls_api.c > @@ -1582,7 +1582,7 @@ static inline int __tcf_classify(struct sk_buff *skb, > u32 *last_executed_chain) > { > #ifdef CONFIG_NET_CLS_ACT > - const int max_reclassify_loop = 4; > + const int max_reclassify_loop = 15; > const struct tcf_proto *first_tp; > int limit = 0; > >
> From: Tim Gardner <tim.gardner@canonical.com> > Sent: Friday, May 7, 2021 7:58 AM > > I hate to be nit picky, but the subject line reclassify loop limit doesn't match > the value in the patch. My mistake there. We initially set the value to 10, but then updated it, and I missed updating the title when I squashed the fixup. > > What kind of performance impact will this incur ? Are unclassifiable packets > common ? Roi, can you answer this? > > On 5/5/21 10:43 AM, Daniel Jurgens wrote: > > From: Roi Dayan <roid@nvidia.com> > > > > BugLink: https://bugs.launchpad.net/bugs/1927257 > > > > To support packet reclassification of more than 4 chains increase > > max_reclassify_loop from 4 to 15. > > > > Change-Id: Ie18c37fb0255a737dc8f1b2fcf10a1163266dde6 > > Signed-off-by: Roi Dayan <roid@nvidia.com> > > Reviewed-by: Oz Shlomo <ozsh@nvidia.com> > > Signed-off-by: Daniel Jurgens <danielj@nvidia.com> > > --- > > net/sched/cls_api.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index > > b6e0e15..9c9afc0 100644 > > --- a/net/sched/cls_api.c > > +++ b/net/sched/cls_api.c > > @@ -1582,7 +1582,7 @@ static inline int __tcf_classify(struct sk_buff *skb, > > u32 *last_executed_chain) > > { > > #ifdef CONFIG_NET_CLS_ACT > > - const int max_reclassify_loop = 4; > > + const int max_reclassify_loop = 15; > > const struct tcf_proto *first_tp; > > int limit = 0; > > > > > > -- > ----------- > Tim Gardner > Canonical, Inc
On 2021-05-07 4:53 PM, Daniel Jurgens wrote: >> From: Tim Gardner <tim.gardner@canonical.com> >> Sent: Friday, May 7, 2021 7:58 AM >> >> I hate to be nit picky, but the subject line reclassify loop limit doesn't match >> the value in the patch. > > My mistake there. We initially set the value to 10, but then updated it, and I missed updating the title when I squashed the fixup. > >> >> What kind of performance impact will this incur ? Are unclassifiable packets >> common ? > > Roi, can you answer this? > Hi, For the general case there is no performance impact. This limit protects against accidental loop in tc rules with goto chains actions. so the only performance impact will only be for the case you actually have rules with many chains and gotos to jump between the chains and only for packets that will actually match the tc rules and do the actions. i.e. chain 0: match A goto chain 1 chain 1: match B goto chain 2 chain 2: match C goto chain 3 etc This is not common but some customers doing connection tracking rules and changing ct zones could use more than the default. Thanks, Roi >> >> On 5/5/21 10:43 AM, Daniel Jurgens wrote: >>> From: Roi Dayan <roid@nvidia.com> >>> >>> BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fbugs%2F1927257&data=04%7C01%7Croid%40nvidia.com%7Cfb101bde402a456f8f6b08d9115f800d%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637559924127140735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=SPKlsmNeNxi%2BA79xVy41Tl8rdk0dDJjIR6O6BdJORBQ%3D&reserved=0 >>> >>> To support packet reclassification of more than 4 chains increase >>> max_reclassify_loop from 4 to 15. >>> >>> Change-Id: Ie18c37fb0255a737dc8f1b2fcf10a1163266dde6 >>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>> Reviewed-by: Oz Shlomo <ozsh@nvidia.com> >>> Signed-off-by: Daniel Jurgens <danielj@nvidia.com> >>> --- >>> net/sched/cls_api.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index >>> b6e0e15..9c9afc0 100644 >>> --- a/net/sched/cls_api.c >>> +++ b/net/sched/cls_api.c >>> @@ -1582,7 +1582,7 @@ static inline int __tcf_classify(struct sk_buff *skb, >>> u32 *last_executed_chain) >>> { >>> #ifdef CONFIG_NET_CLS_ACT >>> - const int max_reclassify_loop = 4; >>> + const int max_reclassify_loop = 15; >>> const struct tcf_proto *first_tp; >>> int limit = 0; >>> >>> >> >> -- >> ----------- >> Tim Gardner >> Canonical, Inc
On 2021-05-10 12:48 PM, Roi Dayan wrote: > > > On 2021-05-07 4:53 PM, Daniel Jurgens wrote: >>> From: Tim Gardner <tim.gardner@canonical.com> >>> Sent: Friday, May 7, 2021 7:58 AM >>> >>> I hate to be nit picky, but the subject line reclassify loop limit >>> doesn't match >>> the value in the patch. >> >> My mistake there. We initially set the value to 10, but then updated >> it, and I missed updating the title when I squashed the fixup. >> >>> >>> What kind of performance impact will this incur ? Are unclassifiable >>> packets >>> common ? >> >> Roi, can you answer this? >> > > Hi, > > For the general case there is no performance impact. > This limit protects against accidental loop in tc rules with goto chains > actions. so the only performance impact will only be for the case you > actually have rules with many chains and gotos to jump between the > chains and only for packets that will actually match the tc rules and > do the actions. > i.e. > > chain 0: match A goto chain 1 > chain 1: match B goto chain 2 > chain 2: match C goto chain 3 > etc > > This is not common but some customers doing connection tracking > rules and changing ct zones could use more than the default. > > Thanks, > Roi > I will also add that the "performance" for allowing more chains reclassifying is vs TC not supporting this case at all. So using openvswitch, those rules will fail to be added to tc )and also to hw eventually when supported) and the matching and handling of those packets will be in ovs datapath. >>> >>> On 5/5/21 10:43 AM, Daniel Jurgens wrote: >>>> From: Roi Dayan <roid@nvidia.com> >>>> >>>> BugLink: >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fbugs%2F1927257&data=04%7C01%7Croid%40nvidia.com%7Cfb101bde402a456f8f6b08d9115f800d%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637559924127140735%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=SPKlsmNeNxi%2BA79xVy41Tl8rdk0dDJjIR6O6BdJORBQ%3D&reserved=0 >>>> >>>> >>>> To support packet reclassification of more than 4 chains increase >>>> max_reclassify_loop from 4 to 15. >>>> >>>> Change-Id: Ie18c37fb0255a737dc8f1b2fcf10a1163266dde6 >>>> Signed-off-by: Roi Dayan <roid@nvidia.com> >>>> Reviewed-by: Oz Shlomo <ozsh@nvidia.com> >>>> Signed-off-by: Daniel Jurgens <danielj@nvidia.com> >>>> --- >>>> net/sched/cls_api.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index >>>> b6e0e15..9c9afc0 100644 >>>> --- a/net/sched/cls_api.c >>>> +++ b/net/sched/cls_api.c >>>> @@ -1582,7 +1582,7 @@ static inline int __tcf_classify(struct >>>> sk_buff *skb, >>>> u32 *last_executed_chain) >>>> { >>>> #ifdef CONFIG_NET_CLS_ACT >>>> - const int max_reclassify_loop = 4; >>>> + const int max_reclassify_loop = 15; >>>> const struct tcf_proto *first_tp; >>>> int limit = 0; >>>> >>>> >>> >>> -- >>> ----------- >>> Tim Gardner >>> Canonical, Inc
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index b6e0e15..9c9afc0 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -1582,7 +1582,7 @@ static inline int __tcf_classify(struct sk_buff *skb, u32 *last_executed_chain) { #ifdef CONFIG_NET_CLS_ACT - const int max_reclassify_loop = 4; + const int max_reclassify_loop = 15; const struct tcf_proto *first_tp; int limit = 0;