Re: [RFC] Add support for hardware transmit rate limiting queues [WAS: Add support for changing the flow ID of TCP connections]

Hi Luigi,

On 08/20/14 11:32, Luigi Rizzo wrote:
> On Wed, Aug 20, 2014 at 9:34 AM, Hans Petter Selasky <hps_at_selasky.org>
> wrote:
>
>> Hi,
>>
>> A month has passed since the last e-mail on this topic, and in the
>> meanwhile some new patches have been created and tested:
>>
>> Basically the approach has been changed a little bit:
>>
>> - The creation of hardware transmit rings has been made independent of the
>> TCP stack. This allows firewall applications to forward traffic into
>> hardware transmit rings aswell, and not only native TCP applications. This
>> should be one more reason to get the feature into the kernel.
>>
>> - A hardware transmit ring basically can have two modes: FIXED-RATE or
>> AUTOMATIC-RATE. In the fixed rate mode all traffic is sent at a fixed bytes
>> per second rate. In the automatic mode you can configure a time after which
>> the TX queue must be empty. The hardware driver uses this to configure the
>> actual rate. In automatic mode you can also set an upper and lower transmit
>> rate limit.
>>
>> - The MBUF has got a new field in the packet header: "txringid"
>>
>> - IOCTLs for TCP v4 and v6 sockets has been updated to allow setting of
>> the "txringid" field in the mbuf.
>>
>> The current patch [see attachment] should be much simpler and less
>> intrusive than the previous one.
>>
>
> ​the patch seems to include only part of the generic code (ie no ioctls
> for manipulating the rates, no backend code). Do i miss something ?

The IOCTLs for managing the rates are:

SIOCARATECTL, SIOCSRATECTL, SIOCGRATECTL and SIOCDRATECTL

And they go to the if_ioctl callback.

>
> I have a few comments/concerns:
>
> + looks like flowid and txringid are overlapped in scope,
>    both will be used (in the backend) to select a specific
>    tx queue. I don't have a solution but would like to know
>    how do you plan to address this -- does one have priority
>    over the other, etc.

Not 100% . In some cases the flowID is used differently than the 
txringid, though it might be possible to join the two. Would need to 
investigate current users of the flow ID.

> + related to the above, a (possibly unavoidable) side effect
>    of this type of changes is that mbufs explode with custom fields,
>    so if we could perhaps make one between flowid and txringid,
>    that would be useful.

Right, but ratecontrol is an in-general useful feature, especially for 
high throughput networks, or do you think otherwise?

>
> + is there a way to ​avoid the replicated code for SIOCSTXRINGID
>    (the ioctl handler, i suppose). Maybe make one function and
>    call it from both ipv4 and ipv6, assuming there aren't other
>    places like this.

Yes, could do that.

>
> + i am not particularly happy about the explosion of ioctls for
>    setting and getting rates. Next we'll want to add scheduling,
>    and intervals, and queue sizes and so on.
>    For these commands outside the critical path it would be
>    preferable a single command with an extensible structure.
>    Bikeshed material i am sure.

There is only one IOCTL in the critical path and that is the IOCTL to 
change or update the TX ring ID. The other IOCTLs are in the 
non-critical path towards the if_ioctl() callback.

If we can merge the flowID and the txringid into one field, would it be 
acceptable to add an IOCTL to read/write this value for all sockets?

--HPS