Page MenuHomeFreeBSD

Add support for generic backpressure indicator for ratelimited transmit queues aswell as non-ratelimited ones
ClosedPublic

Authored by hselasky on Jul 7 2017, 11:16 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 24, 3:22 PM
Unknown Object (File)
Fri, Dec 20, 7:03 PM
Unknown Object (File)
Fri, Dec 20, 6:55 PM
Unknown Object (File)
Fri, Dec 20, 6:54 PM
Unknown Object (File)
Tue, Dec 17, 10:43 AM
Unknown Object (File)
Nov 24 2024, 10:14 PM
Unknown Object (File)
Nov 24 2024, 4:56 PM
Unknown Object (File)
Nov 20 2024, 8:54 PM
Subscribers

Details

Summary

Add the required structure bits in order to support a backpressure indication with ratelimited connections aswell as non-ratelimited ones. The backpressure indicator is a value between zero and 65535 inclusivly, indicating if the destination transmit queue is empty or full respectivly. Applications can use this value as a decision point for when to stop transmitting data to avoid endless ENOBUFS error codes upon transmitting an mbuf. This indicator is also useful to reduce the latency for ratelimited queues.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 10369

Event Timeline

This revision is now accepted and ready to land.Jul 10 2017, 2:13 PM
kib added inline comments.
sys/net/if_var.h
202

Why using uint32_t for a value declared to be 16bit ? Do you have plans for extending the values ?

Also, is this structure ever becomes the part of the user ABI ? Note that on 64bit platforms and on some 32bit plaftorms like ARMv6/7 there is implicit padding at the end which makes the structure to require translation for compat32.

sys/netinet/in_pcb.c
2820

Why not

    if (ifp->if_snd_tag_query == NULL)
	  return (EOPNOTSUPP);

This reduces indent level for the ifp snd_tag_query call below and is in style of earlier code in the function.

Instead of using tags have you considered a modified if_transmit that returns the current occupancy-level of the tx queue (as a percentage or some normalized representation)? That would make it fully stateless and we wouldn't have to involve any inp in any of this. Any feature hooked up to inp will work only for IP connections that terminate on that node and not, say, packet forwarding use cases.

Instead of using tags have you considered a modified if_transmit that returns the current occupancy-level of the tx queue (as a percentage or some normalized representation)? That would make it fully stateless and we wouldn't have to involve any inp in any of this. Any feature hooked up to inp will work only for IP connections that terminate on that node and not, say, packet forwarding use cases.

@np : the current implementation has been very carefully thought through and yes, modifying if_transmit has been considered. Modifying if_transmit doesn't solve the problem, because the occupancy-level of the TX queue is not know until _after_ transmit and we need to know the value _before_ transmit. Relying on the current send tag implementation seems the most sensible involving the least number of races and overhead.

sys/net/if_var.h
202

Yes, you are right. I'll fix.

sys/netinet/in_pcb.c
2820

I'll fix. Thank you!

This revision now requires review to proceed.Sep 6 2017, 6:59 AM

Can everyone have a look at this patch again and press accept ? I want to push this patch unless there are any major blockers.

sys/net/if_var.h
202

It is not so common to use uint16_t in these kind of structures and I think access uint32_t is more efficient when it comes to performance.

It looks fine to me, but this is a view of the person who knows nothing about the network stack. I.e. it seems to be fine WRT ABI proposed or generic code flow.

This revision is now accepted and ready to land.Sep 6 2017, 7:15 AM
This revision was automatically updated to reflect the committed changes.