Rare project developers

[Alexander Gall <>]

Hi Frederic

On Wed, 23 Feb 2022 17:12:06 +0100, Frédéric LOUI <> 
said:

> This is the first counter we implemented and if you pay attention this is 
> the only counter linked to an ingress/egress control
> When we tested it we mc36, IIRC we observed that traffic 
> (stats_pkt_out["$COUNTER_SPEC_PKTS"]) was counted twice. 
> (Not sure if it has an incidence but at that time there was no egress 
> control.)

I didn't check how the p4 code looked back then. I'm trying to
understand the current code (a little bit of comments in the code
would help, BTW :) I find it a bit hard to follow, so please correct
me if I'm wrong with the following analysis.

When a tagged packet arrives on a regular port, ig_md.ingress_id is
set to ig_intr_md.ingress_port in the parser. In
IngressControlVlanIn(), the ingress_port and vlan ID are translated to
a pseudo-port ID (source_id), which is used as an index into the
vlan_in.stats counter array.  That's the counter used as "stats_in" in
the Python SNMP code.

When a tagged packet arrives on the CPU port, ig_md.ingress_id is set
to 0 in the parser. When the packet passes through
IngressControlVlanIn(), the lookup in tbl_vlan_in doesn't have a match
(there is no entry for the tuple (0, <vlanID>), the default action
sets ig_md.source_id to 0 (copy from inress_id) and the packet is
attributed to the "stats_in" counter with index 0.  I think we really
don't need that counter at all, but the important thing is that the
packet is *not* added to the "real" counter for packets received on
the physical port for that VLAN.

After that, the packet passes through

        if (ig_intr_md.ingress_port == CPU_PORT) {
            pkt_out_stats.count(ig_md.source_id);
            ig_tm_md.ucast_egress_port = (PortId_t)hdr.cpu.port;
            ig_tm_md.bypass_egress = 1;
            hdr.cpu.setInvalid();
        }

So, the packet is also counted in the pkt_out_stats counter with index
0, which seems wrong to me, since the information about the VLAN tag
is now lost and all VLANs are counted by the same counter.

The code for the SNMP counter does *not* need to correct tha vlan_in
counter, I believe.

However, it would be correct to fix the vlan_out counter, but this
doesn't work as intended because pkt_out_stats is not really
maintained per VLAN.

A possible fix would be to index pkt_out_stats with the VLAN ID
instead of the source_id (which is always 0). It would be even nicer
if we could fix the counter already in the P4 code rather than leaving
this to the forwarder. One problem with this is that we have to
replicate the counter-fix in multiple places, i.e. the SNMP code and
wherever the forwarder pushes the same counter to freerouter.

-- 
Alex


>> I guess there is similar logic for the counters reported to
>> freerouter. How is this case handled there?
> Well, the other counters are a bit different as they are linked to a table.
> Such as this one:
> https://bitbucket.software.geant.org/projects/RARE/repos/rare/browse/p4src/include/ig_ctl_nat.p4#106

> @mc36, do you remember the exact reason why we did this ?

> Also maybe this is the right moment to harmonise counters and just use 
> counter with table and not control ?

>> Le 23 févr. 2022 à 16:34, Alexander Gall <> a écrit :
>> 
>> Hello Frederic
>> 
>> I'm trying to understand the logic of this code:
>> 
>> self.subif_counters.update(
>> {
>> counter_id: [
>> stats_in["$COUNTER_SPEC_PKTS"]
>> - stats_pkt_out["$COUNTER_SPEC_PKTS"],
>> stats_in["$COUNTER_SPEC_BYTES"]
>> - stats_pkt_out["$COUNTER_SPEC_BYTES"],
>> stats_out["$COUNTER_SPEC_PKTS"]
>> + stats_pkt_out["$COUNTER_SPEC_PKTS"],
>> stats_out["$COUNTER_SPEC_BYTES"]
>> + stats_pkt_out["$COUNTER_SPEC_BYTES"],
>> ]
>> }
>> )
>> 
>> stats_{in,out} are the counters from ig_ctl_vlan_in and
>> eg_ctl_vlan_out and stats_pkt_out is the counter for packets coming in
>> from the CPU port. Those packets are sent directly to their egress
>> port without passing through eg_ctl_vlan_out, i.e. they will not be
>> accounted for in stats_out. This is corrected for by adding
>> stats_pkt_out to stats_out, so I understand that part.
>> 
>> However, I don't understand why we should subtract those packets from
>> stats_in since they were never counted there in the first place.
>> 
>> I'm asking you because I believe that the sub-interface code was
>> written by you and maybe you had a reason for it that I'm not seeing
>> :)
>> 
>> I guess there is similar logic for the counters reported to
>> freerouter. How is this case handled there?
>> 
>> -- 
>> Alex