[v1] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate

[v1] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate

[David Dai]

For high speed adapter like Mellanox CX-5 card, it can reach upto
100 Gbits per second bandwidth. Currently htb already supports 64bit rate
in tc utility. However police action rate and peakrate are still limited
to 32bit value (upto 32 Gbits per second). Add 2 new attributes
TCA_POLICE_RATE64 and TCA_POLICE_RATE64 in kernel for 64bit support
so that tc utility can use them for 64bit rate and peakrate value to
break the 32bit limit, and still keep the backward binary compatibility.

Tested-by: David Dai <zdai@linux.vnet.ibm.com>
Signed-off-by: David Dai <zdai@linux.vnet.ibm.com>
---
 include/uapi/linux/pkt_cls.h |    2 ++
 net/sched/act_police.c       |   27 +++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index b057aee..eb4ea4d 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -159,6 +159,8 @@ enum {
 	TCA_POLICE_AVRATE,
 	TCA_POLICE_RESULT,
 	TCA_POLICE_TM,
+	TCA_POLICE_RATE64,
+	TCA_POLICE_PEAKRATE64,
 	TCA_POLICE_PAD,
 	__TCA_POLICE_MAX
 #define TCA_POLICE_RESULT TCA_POLICE_RESULT
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 49cec3e..ed5372e 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -40,6 +40,8 @@ static int tcf_police_walker(struct net *net, struct sk_buff *skb,
 	[TCA_POLICE_PEAKRATE]	= { .len = TC_RTAB_SIZE },
 	[TCA_POLICE_AVRATE]	= { .type = NLA_U32 },
 	[TCA_POLICE_RESULT]	= { .type = NLA_U32 },
+	[TCA_POLICE_RATE64]     = { .type = NLA_U64 },
+	[TCA_POLICE_PEAKRATE64] = { .type = NLA_U64 },
 };
 
 static int tcf_police_init(struct net *net, struct nlattr *nla,
@@ -58,6 +60,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	struct tcf_police_params *new;
 	bool exists = false;
 	u32 index;
+	u64 rate64, prate64;
 
 	if (nla == NULL)
 		return -EINVAL;
@@ -155,14 +158,18 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	}
 	if (R_tab) {
 		new->rate_present = true;
-		psched_ratecfg_precompute(&new->rate, &R_tab->rate, 0);
+		rate64 = tb[TCA_POLICE_RATE64] ?
+			 nla_get_u64(tb[TCA_POLICE_RATE64]) : 0;
+		psched_ratecfg_precompute(&new->rate, &R_tab->rate, rate64);
 		qdisc_put_rtab(R_tab);
 	} else {
 		new->rate_present = false;
 	}
 	if (P_tab) {
 		new->peak_present = true;
-		psched_ratecfg_precompute(&new->peak, &P_tab->rate, 0);
+		prate64 = tb[TCA_POLICE_PEAKRATE64] ?
+			  nla_get_u64(tb[TCA_POLICE_PEAKRATE64]) : 0;
+		psched_ratecfg_precompute(&new->peak, &P_tab->rate, prate64);
 		qdisc_put_rtab(P_tab);
 	} else {
 		new->peak_present = false;
@@ -313,10 +320,22 @@ static int tcf_police_dump(struct sk_buff *skb, struct tc_action *a,
 				      lockdep_is_held(&police->tcf_lock));
 	opt.mtu = p->tcfp_mtu;
 	opt.burst = PSCHED_NS2TICKS(p->tcfp_burst);
-	if (p->rate_present)
+	if (p->rate_present) {
 		psched_ratecfg_getrate(&opt.rate, &p->rate);
-	if (p->peak_present)
+		if ((police->params->rate.rate_bytes_ps >= (1ULL << 32)) &&
+		    nla_put_u64_64bit(skb, TCA_POLICE_RATE64,
+				      police->params->rate.rate_bytes_ps,
+				      TCA_POLICE_PAD))
+			goto nla_put_failure;
+	}
+	if (p->peak_present) {
 		psched_ratecfg_getrate(&opt.peakrate, &p->peak);
+		if ((police->params->peak.rate_bytes_ps >= (1ULL << 32)) &&
+		    nla_put_u64_64bit(skb, TCA_POLICE_PEAKRATE64,
+				      police->params->peak.rate_bytes_ps,
+				      TCA_POLICE_PAD))
+			goto nla_put_failure;
+	}
 	if (nla_put(skb, TCA_POLICE_TBF, sizeof(opt), &opt))
 		goto nla_put_failure;
 	if (p->tcfp_result &&
-- 
1.7.1

Re: [v1] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate

[Eric Dumazet]

On 8/29/19 12:51 AM, David Dai wrote:
> For high speed adapter like Mellanox CX-5 card, it can reach upto
> 100 Gbits per second bandwidth. Currently htb already supports 64bit rate
> in tc utility. However police action rate and peakrate are still limited
> to 32bit value (upto 32 Gbits per second). Add 2 new attributes
> TCA_POLICE_RATE64 and TCA_POLICE_RATE64 in kernel for 64bit support
> so that tc utility can use them for 64bit rate and peakrate value to
> break the 32bit limit, and still keep the backward binary compatibility.
> 
> Tested-by: David Dai <zdai@linux.vnet.ibm.com>
> Signed-off-by: David Dai <zdai@linux.vnet.ibm.com>
> ---
>  include/uapi/linux/pkt_cls.h |    2 ++
>  net/sched/act_police.c       |   27 +++++++++++++++++++++++----
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index b057aee..eb4ea4d 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -159,6 +159,8 @@ enum {
>  	TCA_POLICE_AVRATE,
>  	TCA_POLICE_RESULT,
>  	TCA_POLICE_TM,
> +	TCA_POLICE_RATE64,
> +	TCA_POLICE_PEAKRATE64,
>  	TCA_POLICE_PAD,
>  	__TCA_POLICE_MAX
>  #define TCA_POLICE_RESULT TCA_POLICE_RESULT

Never insert new attributes, as this breaks compatibility with old binaries (including
old kernels)

Keep TCA_POLICE_PAD value the same, thanks.

Re: [v1] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate

[David Z. Dai]

On Thu, 2019-08-29 at 10:32 +0200, Eric Dumazet wrote:
> 
> On 8/29/19 12:51 AM, David Dai wrote:
> > For high speed adapter like Mellanox CX-5 card, it can reach upto
> > 100 Gbits per second bandwidth. Currently htb already supports 64bit rate
> > in tc utility. However police action rate and peakrate are still limited
> > to 32bit value (upto 32 Gbits per second). Add 2 new attributes
> > TCA_POLICE_RATE64 and TCA_POLICE_RATE64 in kernel for 64bit support
> > so that tc utility can use them for 64bit rate and peakrate value to
> > break the 32bit limit, and still keep the backward binary compatibility.
> > 
> > Tested-by: David Dai <zdai@linux.vnet.ibm.com>
> > Signed-off-by: David Dai <zdai@linux.vnet.ibm.com>
> > ---
> >  include/uapi/linux/pkt_cls.h |    2 ++
> >  net/sched/act_police.c       |   27 +++++++++++++++++++++++----
> >  2 files changed, 25 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> > index b057aee..eb4ea4d 100644
> > --- a/include/uapi/linux/pkt_cls.h
> > +++ b/include/uapi/linux/pkt_cls.h
> > @@ -159,6 +159,8 @@ enum {
> >  	TCA_POLICE_AVRATE,
> >  	TCA_POLICE_RESULT,
> >  	TCA_POLICE_TM,
> > +	TCA_POLICE_RATE64,
> > +	TCA_POLICE_PEAKRATE64,
> >  	TCA_POLICE_PAD,
> >  	__TCA_POLICE_MAX
> >  #define TCA_POLICE_RESULT TCA_POLICE_RESULT
> 
> Never insert new attributes, as this breaks compatibility with old binaries (including
> old kernels)
Thanks for reviewing it!
My change is only contained within the police part. I am trying to
follow the same way htb and tbf support their 64 bit rate.

I tested the old tc binary with the newly patched kernel. It works fine.

I agree the newly compiled tc binary that has these 2 new attributes can
cause backward compatibility issue when running on the old kernel.

If can't insert new attribute, is there any
comment/suggestion/alternative on how to support 64bit police rate and
still keep the backward compatibility?

> Keep TCA_POLICE_PAD value the same, thanks.

Re: [v1] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate

[Toke Høiland-Jørgensen]

"David Z. Dai" <zdai@linux.vnet.ibm.com> writes:

> On Thu, 2019-08-29 at 10:32 +0200, Eric Dumazet wrote:
>> 
>> On 8/29/19 12:51 AM, David Dai wrote:
>> > For high speed adapter like Mellanox CX-5 card, it can reach upto
>> > 100 Gbits per second bandwidth. Currently htb already supports 64bit rate
>> > in tc utility. However police action rate and peakrate are still limited
>> > to 32bit value (upto 32 Gbits per second). Add 2 new attributes
>> > TCA_POLICE_RATE64 and TCA_POLICE_RATE64 in kernel for 64bit support
>> > so that tc utility can use them for 64bit rate and peakrate value to
>> > break the 32bit limit, and still keep the backward binary compatibility.
>> > 
>> > Tested-by: David Dai <zdai@linux.vnet.ibm.com>
>> > Signed-off-by: David Dai <zdai@linux.vnet.ibm.com>
>> > ---
>> >  include/uapi/linux/pkt_cls.h |    2 ++
>> >  net/sched/act_police.c       |   27 +++++++++++++++++++++++----
>> >  2 files changed, 25 insertions(+), 4 deletions(-)
>> > 
>> > diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>> > index b057aee..eb4ea4d 100644
>> > --- a/include/uapi/linux/pkt_cls.h
>> > +++ b/include/uapi/linux/pkt_cls.h
>> > @@ -159,6 +159,8 @@ enum {
>> >  	TCA_POLICE_AVRATE,
>> >  	TCA_POLICE_RESULT,
>> >  	TCA_POLICE_TM,
>> > +	TCA_POLICE_RATE64,
>> > +	TCA_POLICE_PEAKRATE64,
>> >  	TCA_POLICE_PAD,
>> >  	__TCA_POLICE_MAX
>> >  #define TCA_POLICE_RESULT TCA_POLICE_RESULT
>> 
>> Never insert new attributes, as this breaks compatibility with old binaries (including
>> old kernels)
> Thanks for reviewing it!
> My change is only contained within the police part. I am trying to
> follow the same way htb and tbf support their 64 bit rate.
>
> I tested the old tc binary with the newly patched kernel. It works fine.
>
> I agree the newly compiled tc binary that has these 2 new attributes can
> cause backward compatibility issue when running on the old kernel.
>
> If can't insert new attribute, is there any
> comment/suggestion/alternative on how to support 64bit police rate and
> still keep the backward compatibility?

Just put the new attributes *after* PAD instead of before :)

-Toke