Fix CalcRtable array parameter bug #259
Conversation
|
CI is now fixed, but it looks like you need to update the test as well. |
|
@vishvananda |
|
https://github.com/torvalds/linux/blob/master/net/sched/act_police.c#L265 Linux kernel does not return "TCA_POLICE_RATE" in |
| @@ -370,14 +370,6 @@ func TestFilterFwAddDel(t *testing.T) { | |||
| if fw.Police.Rate.Rate != filter.Police.Rate.Rate { | |||
| t.Fatal("Police Rate doesn't match") | |||
| } | |||
| for i := range fw.Rtab { | |||
There was a problem hiding this comment.
So this test was passing because the two arrays fw.Rtab and filter.Rtab were actually empty because of the bug, correct ?
|
@yandd Please squash the two commits in one (they are about the same thing) and sign the commit. |
class_linux.go
Outdated
| @@ -141,12 +141,12 @@ func classPayload(req *nl.NetlinkRequest, class Class) error { | |||
| var rtab [256]uint32 | |||
| var ctab [256]uint32 | |||
| tcrate := nl.TcRateSpec{Rate: uint32(htb.Rate)} | |||
| if CalcRtable(&tcrate, rtab, cellLog, uint32(mtu), linklayer) < 0 { | |||
| if CalcRtable(&tcrate, &rtab, cellLog, uint32(mtu), linklayer) < 0 { | |||
There was a problem hiding this comment.
instead of passing array by reference better use slices: rtab[:]
filter_linux.go
Outdated
| @@ -552,7 +552,7 @@ func AdjustSize(sz uint, mpu uint, linklayer int) uint { | |||
| } | |||
| } | |||
|
|
|||
| func CalcRtable(rate *nl.TcRateSpec, rtab [256]uint32, cellLog int, mtu uint32, linklayer int) int { | |||
| func CalcRtable(rate *nl.TcRateSpec, rtab *[256]uint32, cellLog int, mtu uint32, linklayer int) int { | |||
filter_linux.go
Outdated
| @@ -567,7 +567,7 @@ func CalcRtable(rate *nl.TcRateSpec, rtab [256]uint32, cellLog int, mtu uint32, | |||
| } | |||
| for i := 0; i < 256; i++ { | |||
| sz = AdjustSize(uint((i+1)<<uint32(cellLog)), uint(mpu), linklayer) | |||
| rtab[i] = uint32(Xmittime(uint64(bps), uint32(sz))) | |||
| (*rtab)[i] = uint32(Xmittime(uint64(bps), uint32(sz))) | |||
class_linux.go
Outdated
| return errors.New("HTB: failed to calculate rate table") | ||
| } | ||
| opt.Rate = tcrate | ||
| tcceil := nl.TcRateSpec{Rate: uint32(htb.Ceil)} | ||
| if CalcRtable(&tcceil, ctab, ccellLog, uint32(mtu), linklayer) < 0 { | ||
| if CalcRtable(&tcceil, &ctab, ccellLog, uint32(mtu), linklayer) < 0 { |
|
LGTM |
|
Thanks @yandd @fcrisciani LGTM as well. |
No description provided.