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 <[email protected]>
Signed-off-by: David Dai <[email protected]>
---
Changelog:
v1->v2:
- Move 2 attributes TCA_POLICE_RATE64 TCA_POLICE_PEAKRATE64 after
TCA_POLICE_PAD in pkt_cls.h header.
---
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..a6aa466 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -160,6 +160,8 @@ enum {
TCA_POLICE_RESULT,
TCA_POLICE_TM,
TCA_POLICE_PAD,
+ TCA_POLICE_RATE64,
+ TCA_POLICE_PEAKRATE64,
__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..425f2a3 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_MAX))
+ 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_MAX))
+ 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
On Fri, Aug 30, 2019 at 12:06 PM David Dai <[email protected]> wrote:
> - 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_MAX))
I think the last parameter should be TCA_POLICE_PAD.
On Fri, 2019-08-30 at 12:11 -0700, Cong Wang wrote:
> On Fri, Aug 30, 2019 at 12:06 PM David Dai <[email protected]> wrote:
> > - 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_MAX))
>
> I think the last parameter should be TCA_POLICE_PAD.
Thanks for reviewing it!
I have the impression that last parameter num value should be larger
than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
case). This is the reason I changed the last parameter value to
__TCA_POLICE_MAX after I moved the new attributes after TC_POLICE_PAD in
pkt_cls.h header.
I rebuilt the kernel module act_police.ko by using TC_POLICE_PAD in the
4 parameter as before, I am able to set > 32bit rate and peakrate value
in tc command. It also works properly.
If the rest of community thinks I should keep using TC_POLICE_PAD in the
4th parameter too, I can change it to TC_POLICE_PAD in the next version.
Thanks!
On Fri, Aug 30, 2019 at 1:03 PM David Z. Dai <[email protected]> wrote:
>
> On Fri, 2019-08-30 at 12:11 -0700, Cong Wang wrote:
> > On Fri, Aug 30, 2019 at 12:06 PM David Dai <[email protected]> wrote:
> > > - 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_MAX))
> >
> > I think the last parameter should be TCA_POLICE_PAD.
> Thanks for reviewing it!
> I have the impression that last parameter num value should be larger
> than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
Why do you have this impression?
> case). This is the reason I changed the last parameter value to
> __TCA_POLICE_MAX after I moved the new attributes after TC_POLICE_PAD in
> pkt_cls.h header.
The prototype clearly shows it must be a padding attribute:
static inline int nla_put_u64_64bit(struct sk_buff *skb, int attrtype,
u64 value, int padattr)
From: "David Z. Dai" <[email protected]>
Date: Fri, 30 Aug 2019 15:03:52 -0500
> I have the impression that last parameter num value should be larger
> than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
> case).
The argument in question is explicitly the "padding" value.
Please explain in detail where you got the impression that the
argument has to be larger?
On Fri, 2019-08-30 at 13:33 -0700, David Miller wrote:
> From: "David Z. Dai" <[email protected]>
> Date: Fri, 30 Aug 2019 15:03:52 -0500
>
> > I have the impression that last parameter num value should be larger
> > than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
> > case).
>
> The argument in question is explicitly the "padding" value.
>
> Please explain in detail where you got the impression that the
> argument has to be larger?
In include/uapi/linux/pkt_sched.h header:
For HTB:
enum {
TCA_HTB_UNSPEC,
TCA_HTB_PARMS,
TCA_HTB_INIT,
TCA_HTB_CTAB,
TCA_HTB_RTAB,
TCA_HTB_DIRECT_QLEN,
TCA_HTB_RATE64, /* <--- */
TCA_HTB_CEIL64, /* <--- */
TCA_HTB_PAD, /* <--- */
__TCA_HTB_MAX,
};
/* TCA_HTB_RATE64,TCA_HTB_CEIL64 are declared *before* TCA_HTB_PAD */
For TBF:
enum {
TCA_TBF_UNSPEC,
TCA_TBF_PARMS,
TCA_TBF_RTAB,
TCA_TBF_PTAB,
TCA_TBF_RATE64, /* <--- */
TCA_TBF_PRATE64, /* <--- */
TCA_TBF_BURST,
TCA_TBF_PBURST,
TCA_TBF_PAD, /* <--- */
__TCA_TBF_MAX,
};
/* TCA_TBF_RATE64, TCA_TBF_PRATE64 are declared *before* TCA_TBF_PAD */
For HTB, in net/sched/sch_htb.c file, htb_dump_class() routine:
if ((cl->rate.rate_bytes_ps >= (1ULL << 32)) &&
nla_put_u64_64bit(skb, TCA_HTB_RATE64,
cl->rate.rate_bytes_ps,
TCA_HTB_PAD))
goto nla_put_failure;
if ((cl->ceil.rate_bytes_ps >= (1ULL << 32)) &&
nla_put_u64_64bit(skb, TCA_HTB_CEIL64,
cl->ceil.rate_bytes_ps,
TCA_HTB_PAD))
goto nla_put_failure;
For TBF, in net/sched/sch_tbf.c file, tbf_dump() routine:
if (q->rate.rate_bytes_ps >= (1ULL << 32) &&
nla_put_u64_64bit(skb, TCA_TBF_RATE64,
q->rate.rate_bytes_ps,
TCA_TBF_PAD))
goto nla_put_failure;
if (tbf_peak_present(q) &&
q->peak.rate_bytes_ps >= (1ULL << 32) &&
nla_put_u64_64bit(skb, TCA_TBF_PRATE64,
q->peak.rate_bytes_ps,
TCA_TBF_PAD))
goto nla_put_failure;
The last parameter used TCA_TBF_PAD, TCA_TBF_PAD are all declared
*after* those attributes.
I am trying to keep it consistent in police part. That's where my
impression is coming from.
Now for suggestion/comment, do you think is it better to add a new PAD
attribute in include/uapi/pkt_cls.h like this:
enum {
TCA_POLICE_UNSPEC,
TCA_POLICE_TBF,
TCA_POLICE_RATE,
TCA_POLICE_PEAKRATE,
TCA_POLICE_AVRATE,
TCA_POLICE_RESULT,
TCA_POLICE_TM,
TCA_POLICE_PAD,
TCA_POLICE_RATE64, /* <--- */
TCA_POLICE_PEAKRATE64, /* <--- */
TCA_POLICE_PAD2, /* <--- new PAD */
__TCA_POLICE_MAX
#define TCA_POLICE_RESULT TCA_POLICE_RESULT
#};
Then use this TCA_POLICE_PAD2 as the last parameter in
nla_put_u64_64bit()?
Thanks!