search

sonic-net / DASH

Disaggregated APIs for SONiC Hosts
Apache License 2.0
80 stars 89 forks source link

Formalize annotated name parsing in DASH. #473

Closed r12f closed 9 months ago

r12f commented 9 months ago

Use a normalized way to parse the annotated name in p4 files.

This change will not change any generated files as shown below.

r12f@r12f-dl380:~/data/code/sonic/DASH/dash-pipeline
$ diff SAI/SAI/experimental/ ~/data/code/sonic/DASH-exp/dash-pipeline/SAI/SAI/experimental/

r12f@r12f-dl380:~/data/code/sonic/DASH/dash-pipeline
$ diff SAI/lib ~/data/code/sonic/DASH-exp/dash-pipeline/SAI/lib
chrispsommers commented 9 months ago

Hi @r12f I appreciate this work, but it might be a step in the wrong direction. I am wondering if we really ought to deprecate the P4 entity naming convention e.g. "@name(x|y)" entirely and follow-through on https://github.com/sonic-net/DASH/issues/276 where we agreed that long-term, we should not use a naming convention at all, instead use explicit annotations e.g. "@Sai[a=b,x=y]".

@marian-pritsak took initial steps towards this in https://github.com/sonic-net/DASH/pull/358. See https://github.com/sonic-net/DASH/blob/main/dash-pipeline/bmv2/README.md#p4-annotations-for-sai-code-generation also.

That being said, if this is just cleanup and uniformization, I see no objections to it, but it is kind of perpetuating a convention we'd rather abandon.

Please take a look at the conversation tail above and let us know what you think!

r12f commented 9 months ago

Hi @chrispsommers , yes. I totally agree with you! This is just a clean up work in case things get even worse and we should definitely move the annotation to use @Sai instead.

And thanks for sharing the docs! Once this is done, I will try to make the work to move these annotations to @Sai as the next step.

chrispsommers commented 9 months ago

in case things get even worse LOL!

OK, with that in mind, I am in favor of this. Thanks for considering taking the annotations in the right direction when time allows. We all appreciate the energy you are bringing to this project!

r12f commented 9 months ago

np at all! and I also really appreciated reviewing this PR in the weekend too!

r12f commented 9 months ago

in case things get even worse LOL!

I cannot believe myself used almost half of day just trying to sort the naming out..... XD