DASH Behavioral Model WG, 21 Sep 2023

Previous Meeting

Next Meeting

⏯

youtube image

►

From YouTube: DASH Behavioral Model WG Sept 21 2023

Description

Detailed Notes:
• Chris and Andy discussed a Service Tunnel feature that Andy was trying to simplify down to 64 bits. Chris suggested filing an Issue and creating a PR to get the discussion going offline. Chris also suggested using blame to see who wrote the code and pinging them in the issue.
• Chris and Vincent discussed an issue with compiling Docker images, which may be related to a race condition. Chris suggested filing an Issue and tagging himself and Kamil.
○ Vincent and Chris discussed a compile issue with Docker that Vincent noticed when he pulled up the latest Dash to rebase. Vincent mentioned that Kamil had made some changes to the 'make' files that could be causing the issue.
○ Chris and Marian discussed a bug in the bmv2 code; discussed the possibility of returning more than one object type, but Marian was unsure if it was possible due to the API signature.
• Chris and Saad discussed a Python-based behavior model that Saad has been working on for the past few weekends. The model is a drop-in replacement for the existing P4 behavior model.
○ Saad has been working on a Python based behavior model to replace the existing P4 model. He has completed the functionality and is now working on integration.
○ Chris and Saad discussed a project involving a drop-in replacement for an existing model, which would be written in Python and have the same interfaces as the current P4 runtime approach.
○ Chris and Marian discussed two issues related to the implementation of the model. The first issue was discussed yesterday and Brian agreed to take another look at it. The second issue had code issues and Marian was cited to help.
• Chris and Marian discussed a code path in the P4 test Gen that was found to be dead code. Marian explained that the NVGRE encap function is agnostic to the overlay IP family and that the service tunnel is before the IPV6 translation.
○ Marian and Chris discussed the code path of NVGRE encap. Marian explained that the function is agnostic to the overlay IP family and that it supports both IPv4 and IPv6 headers.
○ Chris, Andy, and Marian discussed the potential of using a code path as a CICD test case for P4 test gen. Marian suggested that the solution should not be to remove the condition from the function.
• Chris talked about Kamil's work on refactoring the SAI Thrift server in the SAI repo to compile the SAI Meta into a Lib Sai Meta.
○ Chris discussed the progress of Camille's refactoring of the side thrift server in the Sai repo. The goal is to compile the side meta into a Lib Sai Meta, which will eliminate the need for it in the vendor libsai.
○ Chris and Agrawal discussed the status of test cases running on hardware. Vincent offered to check the test cases and provide feedback. Chris suggested that other vendors could also try the test cases on their own implementations.

A

uh What would it be to make this work out? Like you know, all ones in I think it would be uh like St DST mask might be hex. You know eight F's, you know 32 bits of ones, the least never get 32 bits and zeros and the most 96 and significant 96 bits. Perhaps in order to make this statement makes sense, are useful to copy, uh but I don't know, I didn't find any evidence anywhere in the code. It looks like these masks come from the control plane. Perhaps if I remember correctly,.

B

So this is service tunnel feature, and um you want to know if there's a way to like simplify this down to something that could be done in 64 bits. Is that the essence of your question.

A

Or you know other restrictions on the actual intended uses of these masks such that I. Can you know temporarily in a you know, if Deft version on the side of this turn these into just I'm, assuming assignments of the beef, the V4 address to at least maybe 32 bits of the Dust address. If that's what it's doing right.

B

um I, don't know if anyone's on this call, who can actually answer that who's looks like we're. Not. We don't have the experts associated with this feature, but look at my mom speak up, but you know what might be helpful is just to file an issue University issue on this and and see your answers, my flag, probably Martin or whoever you know flagged them. uh Whoever contributed this code we're trying to get the discussion going offline anyway, yep, okay, that's your intention. It would be.

B

It would be handy if we can at least make a simplifying Assumption of some sort. And what would you do something like operate on a half size of this address? um We haven't broken.

A

Enough I mean I can create a pull request, showing the suggested change, but just to ask whether it's it's uh intended to be whether it would be equivalent or not in Behavior, but I have no urgency to want to merge in such a change. I just I could use that as a form of the way to ask the question precisely.

B

That's fine, too, because that way you can see the Delta it's easy to use, get right! Yep! You can do it. You can do sort of an inter in interrogatory pull request. I guess.

C

I do blame on this code, which you'll see at least the owner of that code, because I don't know who wrote that yeah.

A

So I'll I can use that to maybe at ping and GitHub, whoever added this code and the issue. Another pull request thanks. I'll do.

B

That so is this. um This is an example of the new code that was added since the last time. P4 dpdk was, let's say, enhanced to fix the previous bugs. Then this popped out when you tried it on Master, again or Main, yeah.

A

I believe this was something added in the last six months it has been a while, since the last last round of code, I was trying to get working got it.

D

B

Well, no thanks for um bringing it up.

B

um Okay, so we'll we'll file, something um just make a note.

B

By the way, um Christina responded to my request: she off camera started the recording on this. So it's all permanent part of the record um okay. So anyone does anyone else, have any topics they want to bring up. I know, there's a lot of ongoing conversations and I saw some activity yesterday and that may or may not spill into today.

B

um I couldn't make yesterday I.

C

Got a quick thing, but I don't know I, don't know, I'll just throw you there. I, don't know! That's really. It's part of this of the over meeting I just pulled up the latest dash to rebase and um I'm, noticing some compile issues with a Docker like I, usually compile with Dash da on an 80 core uh um x86 machine, and it's fine. But now, when I do this, uh it's complaining about Docker image already being in use so I'm.

C

Thinking that there is this dependency missing somewhere, um it was kind of serialized before I suppose, but not anymore.

A

C

Don't know if Camille or Camille compile he made a lot of changes in make files and whatnot so I wonder why they tried as far as compiling with the most RL CPUs I, suppose.

B

Well, I'm not sure why this would matter on what kind of CPU have sounds like you have a Docker image already running I mean: did you do Docker PS to see? Maybe you didn't do make kill all right.

C

Yeah and there's nothing there, so it's really low. It's really uh a problem when I'm compiling I got zero Docker image running when I'm compiling. There are two processes that run in parallel now didn't use to I suppose in the and that's all in the you know in the dash pipeline, compile right in the make of that um that now a conflicting I haven't I just did the pull the rebate so I don't have any information on that, but um just a side note there.

B

Yeah, if you, if you, can file an issue and and tag myself, then Camille that would be helpful. Yeah.

C

B

Three days.

C

I'm moving on PTO for two weeks, so I just wanted to mention that I'll open an issue and see what I can find in the next two days.

B

Okay, um yeah, thanks for raising that I did. You know because Camille made some substantial changes which, if you ignore this problem for a moment, it's resulted in much faster, build times. Overall, he's really done some good work, um but if there's some side effects we'll have to address them.

C

It's faster. It could be now that the race condition the race condition was already there, but we didn't hit it, but now it's faster, so awesome.

B

Thought I don't know anything that would cause parallel parallelism issues.

C

I'll find an issue and try to investigate for the next two days and we'll see where we go. Yeah.

B

And, and you know, paste some of the console output that shows the you know declaring the error I'd like to see that.

B

E

B

I'm just making some notes.

B

um Okay um sounds like we're collecting some good stuff today. Does anyone else have anything to raise at all bring up some other topics that are ongoing? Hey.

F

uh Hi Chris this.

B

F

Hey good, so uh I had just one thing: that is an issue 441 I think uh Vincent at least that I have responded to it. So I think if it looks good, we can close it.

B

Okay, let me bring that at 441, oh yeah. Can we share my screen.

B

I was just looking at that too trying to catch up a little behind on my uh ongoing.

B

So this is the issue: it's site, API meter, Flags, not mandatory, meter class, yeah, DJ opened it and, let's see no Vincent opened it and BJ responded. So Vincent. Are you okay with this response? Is this still an open issue for you.

C

Yeah I have to go back to it, but it should be fine to close it. I.

B

Think so do you want to have uh you want to hold off on me closing it here and you take a look and, if you're happy with it close it, so you don't have to do it under time pressure here, yeah I'll.

C

Do that I can close the issue so I'll look at it this afternoon. Sure yeah thank.

F

D

C

I kind of related to that and I know it's not P4 oriented here, but the side changes for the mandatory and all that um that's gonna eventually cause if ellipse is following the sci for ACL, for instance, that's going to cause maybe some test case to fail, so I have I'll, probably go through that rewrite, and maybe I'll push an update to you to Dash to align a couple of things. There.

B

Okay, now you're gonna address this in a couple weeks sounds like you're, yeah yeah ready to go okay. So let me just make a note about that too. So, maybe some collaterals in the side test cases.

C

Yeah I think so: I'll get there when I get there but I believe so. Yeah.

B

Okay, great thank you. um Oh hi, Marion I, didn't know you were here must have just joined um thanks for joining us, so actually um Martin since you're. Here Matt wanted to ask a few questions that are open here. People have questions this one in particular, you might be able to give the good answer.

B

It's been um being bandied about for a while, and you know, Camille joined us more recently in this project and he and he was puzzled by the the five ACL tables and objects being created for HCL rules and he's not here so I. Don't think. Let me double check.

E

Yeah I haven't seen this issue by the description is Right books like the bug.

B

Looks like a bug.

E

B

E

Not supposed to return more than or create more than one object.

B

Okay, so you did, you were answering now I thought. Maybe you had created this code way way back when, in my in my misunderstanding, did someone else create this.

E

No I think it wasn't me yeah.

B

Okay, um so what what's the disposition that of this issue? Do you think you it's been asked and answered Myron, or do you need some more time to look at this? What.

E

Do you think I haven't looked at it before I need to uh I need to read through the entire description? Okay, I, don't know if how it's even possible to return more than one object type seems weird, because API signature won't even allow that, but maybe I don't understand something.

E

B

Yeah appreciate your help on this.

B

I, just pinged you an issue so I have a little reminder.

B

B

So um moving along I wanted to ask um side you've been working on on a python-based behavior model. Do you want to update the team about that.

G

Hello, uh can you please, can you hear him.

B

G

Hello yeah so I'm, uh so it was, the functionality was complete and the only thing missing was integration and I've been working on it for last few weekends and I'm almost there, so the python model. uh So basically, there are three attached points that I need to. So one is that I need to create reads which is pretty easy. The second thing is I need to uh Auto, generate the exact same PSI API and implementation from the python model.

G

That part part is also also done so what I did was I took the P4 info file that was generated by the original P4 model and I generated that exact same in extracted, exact, same information in from the python model, and then I fed that then my let's say P4 info file into the script. I did not make any changes in the script and it was generating the exact same PSI headers in implementation. So the only thing left to do is to change the implementation such that instead of following P4 RT apis.

G

It will uh basically call the python model.

G

Such that you can remotely invoke some of the credits, so I made a few crud functions like add. A new entry insert get set Etc and I made a remote, C plus plus interface to do them remotely internally, it uses protobuf to serialize and deserialize, and that is also working so right now. The only thing left to do is to so right now, I'm changing those the ellipsi implementation, Ginger files uh such that instead of calling key for RT. It is using my let's say: python: okay, API.

F

G

Hopefully, we'll start with, you know, also also wrote a stateful test, so I think it will be ready uh next by next week. I will work this weekend, most likely it will be ready by next week.

B

That's very interesting so, just to summarize for people um who may be new to this long live a discussion, he's got a pull request here. That's been up for a while. It's kind of a draft and he's created a from scratch. Python behavior model that mimics all the functionality of the P4 behavior model and its purpose is to be a drop in replacement.

B

That has a lib size that can be I, guess you're, going to link it to the side through server also, so it's really a drop in replacement for the existing behavior model and all the interfaces will be obeyed at the libs high level and at the B level for traffic and he's working on some schemes to you know: Implement all the internals. For that he's, discarding the current P4 runtime approach, that's used for bnb2 and come in with his own and as long as it's a black box that has the same interfaces.

B

That's fine, so it'll be interesting to see how far he can take this. Let's um written directly in Python, it will be a little more easy to play with, let's say than some of the limitations of pure P4 based approach, so I think so kind of a fascinating experiment see how far it goes.

B

They don't have any questions for sod about this one. It's pretty interesting project.

E

A

A question but cool to see the effort being put in and what was it working.

G

B

So I imagine it would be treated as a parallel model. Just like the P4 dpdk is kind of a you know, experiment in progress, and you know if it can pass test cases. I, don't see any reason to not. You know, support its growth and evolution as as nothing else, an interesting experiment and proved out to have a lot of you know. Merit people can can work with it.

B

It's it's once it's built and working and Assad's done all the hard work. If it runs in the pipeline and passes test I'm not going to complain. You know it's free Cycles on Azure Cloud to run the tests, and then we have an interesting another interesting model.

G

Yeah so I just wanted to add one thing earlier in you said that I actually do not write anything from scratch. I translated the existing model, almost. Secondly, yeah. That is the intention such that no tests have to be written. The intention is for you to seamlessly fit into that test and test framework.

G

Hopefully uh you will not need to make any changes anywhere.

B

Well, you call it not writing it from scratch, but if it's brand new python code that follows the exact flavor, I still call it typing from scratch right.

G

Yeah, you can see that so the I I just want to say that the existing effort has been preserved in this yeah I.

B

Understand I I know you're being humble, but it's a lot of work and I'm impressed that you got this far um okay. So um let me just pop through issues and see if there's anything that floats to the top for the purpose of this group's time, any other topics anyone wants to raise at this time.

B

um So as far as issues, we have this one that um Brian agreed to take another look at now. This one was discussed yesterday if I'm not mistaken, I've been since I've saw the discussion thread on that.

C

Yeah, that's correct. um You should be able to close it actually. Okay,.

B

Yeah I think in summary, yeah they're all mandatory, but you can use zero Mass to make them effectively. Don't cares right, not necessarily efficient but based on some of the implementation, say, restrictions of this of this model, um and then this one here I believe Martin. You were also cited and here to try to help on this.

B

And there's there's.

G

Some code issues.

B

I'm wondering if you've had a chance to look at this one Mario issue, 415 yeah,.

E

But this is um These. Warnings uh are not the wrong Behavior. It's just not the best way to to do casting so for now, I'm, not fixing that as I have I was sending clear, requested, I'm working on so for now.

B

It's on hold, you think it would be fairly straightforward just to put a a better cast in there. Is it just a second when.

E

It needs to be tweaked, I, don't know if it would be easy to change, but for now the behavior is not broken. I would say.

B

Okay, all right.

E

Just making one here.

B

B

Thanks for coming on that.

B

Marching through.

B

So Andy you kind of touched on that P4 dpdk by virtue of bringing up that latest issue. That kind of indicates you know it's work in progress and more issues have been uncovered and you'll pass them up right.

A

I'm yeah I'll uh continue testing fixes and against latest P4 and file issues as uh as they come up.

B

Oh here's one um and hopefully Martin you- can actually help us on this.

B

There there's some code.

B

So Fabian Rafi in running P4, test gen found a case that, in the so Andy took some code that had this complex expression.

B

And um he kind of broke it up into sub-expressions in order to fit the P40 pdk, um back-end.

F

B

And this particular code path never gets taken because the code that calls this uh this code guarantees that this is never valid. So this is um nvg. What's the functioning, um this is NV GRE end cap.

E

Yeah yeah but uh I don't understand why um itv6 cannot be entered.

B

No no ipv4, oh.

E

Center, okay, then I don't understand why ipv4 cannot be entered in and with Jerry and.

B

Okay and that's the subject of this, he said NVR, the end Captain only call when route service tunnel was previously accidentally, always invalid.

B

So did you do you have a comment here and how we can resolve this, because it's an interesting find right. We have a dead code path, that's either an error or.

E

It's that called the energy or uncomp is a generic function that would encapsulate any uh it family. It's just not used for Bender V4 right now. Okay,.

B

So it's not used now, but it might be in the future. That's kind of the point here: yeah.

E

I would say it's agnostic to to Overlay.

B

Could you repeat that I didn't quite hear you.

E

It is agnostic to Overlay IP family.

B

Okay, so the fact that we're setting it inbound here is this: something that's going to be true for all time.

B

This is um service tunnel and code, but.

E

That yeah that's service time so service tunnel is, uh is actually before V4 to V6 translation. So right there are two. There are two steps in this uh flow. First is uh we know for sure that this is uh originally before we do translation to V6 and after that we do nvgary angle.

E

So, okay, in this scenario, kind of you always have our lady six, but taking nvidiary and cap alone, or you can compare it with vxline right, we explained end cap uh can encapsulate uh both V4 and V6 headers and it happens in this case. It just doesn't happen or there is no uh path in the code to to having before overlay with nvgre, but I. Don't think uh it's uh that it's really that just we just don't don't use it but uh yeah it is. It supports such a case.

E

D

F

E

Example, if we would want to for Win It to Win It use, NV GRE, okay,.

B

So stepping back then I guess the point of this PR was hey. I ran this test, uh this code code, analysis and I see it would pass- and that's just true today by circumstance in the future, could be not true. Yeah.

E

Technically, it's true I I, agree, um but I think the solution shouldn't be to remove that condition from a function. Yeah.

B

um So Andy, you know maybe a way in here, but I think I think fabian's goal was hey: I want to actually run the dash code as a CI CD test case for my P4 test. Channel I want to you know, because it's a really good real world test of some interesting cases, and he was saying: are you guys going to fix this dead code before I? Add it to the test suite, and maybe the the correct answer is no, it's still a good test case. Today. It looks like dead code, but that's only circumstantial.

B

It may not be dead code tomorrow right and how he wants to deal with. That is up to him right.

A

I think so yeah I mean I. I suspect he I can ask him, but I suspect he passed on this issue. Just in case you know we were unaware.

B

Yeah and and he's pinged us- and you know, uh he's been very good about helping us. You know understand this issue, given that there's been presentations on people or test gen I think it's sitcom or somewhere recently right I, don't think he's you know kind of elevating P4, Destin's visibility, so I wanted to kind of get closure on this.

B

So I think I'll summarize this conversation in the issue and close it um and just say yeah: it is dead today, but tomorrow might not be.

B

um You know how you want to handle that in your use of this as a test case is your call.

A

Yep and and thanks for the report- yeah yeah, yeah yeah.

B

A

Leaves the P4 touchdown is is not erroneously reporting this. It looks correctly reporting this today.

B

Yeah yeah, no, it's actually excellent, actually shed some light on something sort of buried. You know that we wouldn't necessarily know okay cool.

E

Yeah well, basically, I I copied the vxlan and cap to nvidiary mcap and only changed the nvg part. So yeah.

B

Awesome: okay, getting some things done.

E

Sorry I need to drop now. uh So if you have anything else, maybe please leave a message in the.

B

Chat yeah thanks a lot now you have to close some some issues appreciate it thanks, bye um and oh by the way. um Just so people know this topic here, Camille's been working on, it he's actually been refactoring. The sci Thrift server in the psy repo to um separately, compile the sign meta into a live, SCI, meta and, and that way we won't have to have it in the vendor lib size.

B

So Vincent, um you know you raising this issue originally is resulting in quite a bit of action, and uh you know if you're interested in what he's doing just go over to the psy repo. You can see pull requests and whatnot going on.

B

um So, just just to let people know Camille, couldn't it's been um from Microsoft's, been doing a lot of refactoring um PSY and dash with the end goal of having a virtualized dash completely linked into Sonic and sync D. So it's a good effort. It needs finding all kinds of impedance match issues and rectifying them as an expert in this. So it's very helpful kind of lifted. The quality of this Dash project up quite a bit.

B

um I didn't really have I, don't think, there's any pending pull requests that are related to this effort that we need to talk about.

B

uh Oh ashitosh, I I was actually looking at this this morning. Is there anything here that do you need to nudge people to work on this to review it? I have I'll confess, I haven't really taken a deep look at it, but what's maybe you could update us on this one if this is like on your plate and something you wish to move forward.

D

Yeah sure I think we're waiting for the community to review it, uh especially Vijay, and the team which implemented the metering functionality so.

B

Does this, does this run in the behavioral model, or is this only for you know Hardware devices at this point.

D

I think at this point is just for the hardware devices, but.

G

We can try doing.

D

It on the behavioral model also, you know.

B

Yeah uh checks so.

B

D

There was older, PR open, which was not compiling or not passing some of the tests because of uh Arc was passed incorrectly. I have fixed it, okay, so okay, so it's passing on the test today it is the existing ones here,.

B

So all these new tests- uh yeah okay, you're skipping skipping at this bnb2, so I- guess the only way to really verify this, because CI CD is not going to actually run on these new test cases. It's sort of take it on faith. You know that they're good cases. We can visually inspect it, but there's no way like for the team to verify it. But are you um claiming that these run on your hardware and that they pass.

D

Yeah we have done this on Hardware, so okay, but I have to probably check with the team again just to get the current status, but at some point they were. They were running this test when they were written.

B

Okay, so yeah, it puts us in a bit of a quandary, because the behavioral model is really not quite complete, and so it's not easy to like that. We sort of go on a honor System, yet they pass and someday.

B

um Other people be able to try them, um but there is the option for other vendors to try these on their implementation, and you know I'll just you know, there's people in this call who have implementations that they could run it against and if they have an opportunity to try these test cases and their setup. That would be very helpful to then approve the test case.

B

I mean approve, approve the pr they want to weigh in and volunteer their time to do that, and it probably wouldn't be a bad thing because then they get someone else has written a test case and they can try it on their own implementation. It's a good cross check. So does anyone here want to make a comment on that so.

C

The that should be integrated into the usual.

B

Infrastructure right yeah I mean it's just another test case yeah right and.

C

I suppose running the size, Challenger I didn't see it. No actually PTF, okay, yeah yeah.

B

So it thanks for speaking up Vincent. If you want to give these a shot and uh say yeah, you know these test cases, look good.

B

You don't necessarily have to tell us if it works on your Hardware, but if you agree that or agree that the test cases are sound, then we can get someone, because if I read through it all just be not in my head and saying yep, it looks like good python code, but it won't prove anything right.

C

No I I can yeah I'll take those because, typically gay once they get merged, we are basically rebase and pick it up and that's part of a regression that I have anyway.

B

Yeah, if you want to pull this one, hold this branch and give it a sneak preview. That would be super helpful if anyone else from another vendor wants.

E

B

The same it would be great, and then you get the benefit from you know: contribution okay, good, all righty um I, don't have anything else. I wanted to bring up so I'll do a final call or any other topics or issues um people want to discuss before we wrap things up today.

B

Okay, um then, why don't we call this a wrap? It's been real, productive and thanks everybody for coming and we'll see you in two weeks at the next meeting have a great weekend. Everyone.

E

Thanks Chris thanks.

B

Bye-Bye thanks Chris yeah.