Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for setting packets and bytes counter widths separately #119

Open
qobilidop opened this issue Apr 5, 2023 · 5 comments
Open

Proposal for setting packets and bytes counter widths separately #119

qobilidop opened this issue Apr 5, 2023 · 5 comments

Comments

@qobilidop
Copy link
Member

Currently in PNA, a counter extern is defined as the following:

enum PNA_CounterType_t {
    PACKETS,
    BYTES,
    PACKETS_AND_BYTES
}

extern Counter<W, S> {
  Counter(bit<32> n_counters, PNA_CounterType_t type);
  void count(in S index, @optional in bit<32> increment);
}

When type == PNA_CounterType_t.PACKETS_AND_BYTES, the counter is used to count both packets and bytes. The way it works (as I understand) is that some bits of the counter works as the packets counter, while the rest of the bits work as the bytes counter. The issue here is that we can only specify the total width of the combined counter through W, but not the 2 counters separately.

Would it be a good idea to enable setting packets and bytes counter widths separately?

@qobilidop
Copy link
Member Author

qobilidop commented Apr 5, 2023

In PSA spec (C. Appendix: Example implementation of Counter extern), I see the following example and comments:

Counter(bit<32> n_counters, PSA_CounterType_t type) {
    this.num_counters = n_counters;
    this.counter_vals = new array of size n_counters, each element with type W;
    this.type = type;
    if (this.type == PSA_CounterType_t.PACKETS_AND_BYTES) {
        // Packet and byte counts share storage in the same counter
        // state.  Should we have a separate constructor with an
        // additional argument indicating how many of the bits to use
        // for the byte counter?
        W shift_amount = TBD;
        this.shifted_packet_count = ((W) 1) << shift_amount;
        this.packet_count_mask = (~((W) 0)) << shift_amount;
        this.byte_count_mask = ~this.packet_count_mask;
    }
}

So it seems this might have been discussed previously?

@jfingerh
Copy link
Contributor

jfingerh commented Apr 5, 2023

We can check with implementers, but I suspect that specifying the data plane bit width of counters might have been (a) something a lot of targets do not support, because (b) some targets, to save precious on-chip die area, use small counters with either fast enough low-level driver read-and-clearing to guarantee they never wrap, accumulating the full count values in cheaper general purpose CPU DRAM, or in some cases have special hardware mechanisms to "push" such read-and-cleared values of counters that are the largest current values on the chip, in such a way that it prevents loss of count information.

I would not be surprised if most P4 targets simply ignored these bit widths, and used values that were as small as possible, yet wide enough that the particular target's hardware & driver software prevent counter information loss.

@qobilidop
Copy link
Member Author

Do comments (a) and (b) apply to counters in general or is there something specific to the PACKETS_AND_BYTES counters?

If this applies to all counters, does it mean when I read the counter, I could get a value outside the range indicated by the counter bit width? For example, for Counter<bit<8>, bit<1>>(1, PNA_CounterType_t.PACKETS) counter_set, could I read a counter value > 255 if the hardware counter used is wider than 8 bits? My guess is that P4Runtime will (and should) take care of truncating the value to respect the specified type, right?

In the case where hardware cannot support a wide enough counter that the P4 code specifies, I wonder if the compiler could reject such code and give a proper error message? I think this might be a good way to communicate hardware constraints to users.

@jfingerh
Copy link
Contributor

jfingerh commented Apr 5, 2023

Again, we can check with implementers, but I would not be surprised if implementations do NOT make such a restriction on their count values based upon these bit widths in the P4 code.

My comments apply equally for packet counts, byte counts, and packet & byte counts.

@qobilidop
Copy link
Member Author

I see. Thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants