Cong Wang <xiyou.wangcong@xxxxxxxxx> writes: > On Wed, Jul 13, 2022 at 11:52:07PM +0200, Toke Høiland-Jørgensen wrote: >> Stanislav Fomichev <sdf@xxxxxxxxxx> writes: >> >> > On Wed, Jul 13, 2022 at 4:14 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: >> >> >> >> Packet forwarding is an important use case for XDP, which offers >> >> significant performance improvements compared to forwarding using the >> >> regular networking stack. However, XDP currently offers no mechanism to >> >> delay, queue or schedule packets, which limits the practical uses for >> >> XDP-based forwarding to those where the capacity of input and output links >> >> always match each other (i.e., no rate transitions or many-to-one >> >> forwarding). It also prevents an XDP-based router from doing any kind of >> >> traffic shaping or reordering to enforce policy. >> >> >> >> This series represents a first RFC of our attempt to remedy this lack. The >> >> code in these patches is functional, but needs additional testing and >> >> polishing before being considered for merging. I'm posting it here as an >> >> RFC to get some early feedback on the API and overall design of the >> >> feature. >> >> >> >> DESIGN >> >> >> >> The design consists of three components: A new map type for storing XDP >> >> frames, a new 'dequeue' program type that will run in the TX softirq to >> >> provide the stack with packets to transmit, and a set of helpers to dequeue >> >> packets from the map, optionally drop them, and to schedule an interface >> >> for transmission. >> >> >> >> The new map type is modelled on the PIFO data structure proposed in the >> >> literature[0][1]. It represents a priority queue where packets can be >> >> enqueued in any priority, but is always dequeued from the head. From the >> >> XDP side, the map is simply used as a target for the bpf_redirect_map() >> >> helper, where the target index is the desired priority. >> > >> > I have the same question I asked on the series from Cong: >> > Any considerations for existing carousel/edt-like models? >> >> Well, the reason for the addition in patch 5 (continuously increasing >> priorities) is exactly to be able to implement EDT-like behaviour, where >> the priority is used as time units to clock out packets. > > Are you sure? I seriouly doubt your patch can do this at all... > > Since your patch relies on bpf_map_push_elem(), which has no room for > 'key' hence you reuse 'flags' but you also reserve 4 bits there... How > could tstamp be packed with 4 reserved bits?? Well, my point was that the *data structure* itself supports 64-bit priorities, and that's what we use from bpf_map_redirect() in XDP. The choice of reserving four bits was a bit of an arbitrary choice on my part. I actually figured 60 bits would be plenty to represent timestamps in themselves, but I guess I miscalculated a bit for nanosecond timestamps (60 bits only gets you 36 years of range there). We could lower that to 2 reserved bits, which gets you a range of 146 years using 62 bits; or users could just right-shift the value by a couple of bits before putting them in the map (scheduling with single-nanosecond precision is not possible anyway, so losing a few bits of precision is no big deal); or we could add a new helper instead of reusing the existing one. > Actually, if we look into the in-kernel EDT implementation > (net/sched/sch_etf.c), it is also based on rbtree rather than PIFO. The main reason I eschewed the existing rbtree code is that I don't believe it's sufficiently performant, mainly due to the rebalancing. This is a hunch, though, and as I mentioned in a different reply I'm planning to go back and revisit the data structure, including benchmarking different implementations against each other. -Toke