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

Some (most) keys saved by the MJRTY are stored as "-1" #1

Open
gon1995matos opened this issue Feb 20, 2021 · 2 comments
Open

Some (most) keys saved by the MJRTY are stored as "-1" #1

gon1995matos opened this issue Feb 20, 2021 · 2 comments
Assignees
Labels
avoided bug Something isn't working

Comments

@gon1995matos
Copy link
Collaborator

No description provided.

@gon1995matos gon1995matos added the bug Something isn't working label Feb 20, 2021
@gon1995matos gon1995matos self-assigned this Feb 20, 2021
@gon1995matos gon1995matos added the good first issue Good for newcomers label Feb 20, 2021
@gon1995matos
Copy link
Collaborator Author

gon1995matos commented Mar 3, 2021

Hello @signorello,

I have just pushed some changes with the bug I am currently facing in the P4 implementation, to a new branch (bug-fixing).
I would appreciate it very much if you could give me some feedback about this problem.

Commit: 9822080

What is the problem

The majority vote algorithm (mjrty) from the MV Sketch implementation requires storing the five-tuple keys. To avoid re-submitting packets and because I need to compare the stored key with the current one, I am using an array of bit<128> register (sketch_key) to store the srcAddr (32bit), dstAddr (32bit), srcPort (16bit), dstPort (16bit), protocol (8bit) for each bucket.

Everything works well until I have to read the register from the controller. Even if the log shows that a 128-bit value was written into a position in the register, reading the same value from the controller will return "-1".

This happens every time for the 128-bit array register but only happens sometimes when using two 64-bit array registers. Which led me to believe that the controller might be trying to read a 64-bit signed integer from the registers but when it gets more than that it will just return "-1". Could that be the reason? How could I avoid this?

How to replicate the problem

Start the simple switch:

$ p4c --target bmv2 --arch v1model --std p4-16 p4_src/kary.p4
$ sudo $BMV2_DIR/tools/veth_setup.sh
$ sudo simple_switch --log-console -i 0@veth2 -i 1@veth4 --nanolog ipc:///tmp/bm-log.ipc kary.json

Send one packet using the script send.py (press Enter to send a packet):

$ sudo ./send.py ../traces/tcp-syn-100k-1.pcap

Use simple_switch_CLI to read the sketch_key register:

$ simple_switch_CLI
 $ register_read sketch_key

@signorello
Copy link
Collaborator

signorello commented Mar 3, 2021

Hello @gon1995matos

even before looking at your code there are a few conceptual bits here which we would need to clarify.

(i) register-size: you should know that 128-bit hw-registers are rare/nonexistent :) So, you should assume not to have that structure on your target. If you really needed (see (ii) below) to store that amount of bits, you should think about how to split it across multiple registers (of, for example, 32-bit size).
(ii) why are we using the 5-tuple as key and not sth coarser like {srcIP, dstIP}? Do we really need the five-tuple for change detection? Did not we run the experiments in software by simply using the src&dst IPs pair? this is a broader point, since I am not concerned here with the specific implementation problem you are facing right now, but rather with the size of the keys which will need to store together with each bucket and with the comparison operations in the data-plane for the voting algorithm.

gon1995matos added a commit that referenced this issue Mar 4, 2021
Reverts keys to src and dst only. By using a 32-bit register for each, instead of a 64-bit for both, solves the problem with reading values from the controller.

This commit includes some variable renaming to allow for a more intuitive read.
@gon1995matos gon1995matos added avoided and removed good first issue Good for newcomers labels Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
avoided bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants