DASH Behavioral Model WG, 16 Feb 2023

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: DASH Behavioral Model WG Feb 16 2023

Description

More guidance needed for inbound processing.
- Review Logic on a returning packet that had a src IP change

Capture task to not only update the code (PR) but also creating issue that will follow to update HLD

PR#332 Will be merged, all comments resolved. Soon to be Merged
Add #ifdef's so P4 code can target BMv2 or DPDK back ends by jfingerh · Pull Request #332 · sonic-net/DASH (github.com)
- Not only checking pna architecture as well as dpdk

Do we use the same sha tag for both docker images (dpdk & p4c)?

Consider freezing sha for p4c and floating for dpdk & pna
- Upgrade as needed once confirm things are working as expected

A

A

All right today is February 16th, and this is the behavior model. Work group weekly call um we're going to start off by just doing a quick review of what we discussed last week.

A

uh This um be able to admit some books here, all right so last week, um I know that, specifically, we wanted to just go over the behavior mod V2 for SC, slash PL, and we were going to discuss this um today, specifically the 308 uh pull request, but overall we were kind of just going through um the part that we do have working and then figuring out um just that bottom portion and then also figuring out um how to just work through the the top part that we were trying to produce so like this portion right here, um but I think it would be good to just kind of start with the the first thing, which is just like the pull request 308.

A

Unless we have anything else, we want to add to the list for PRS to.

B

A

B

I think Andy's PR is ready to be merged into the the p4p dpdk.

C

I'm, unaware of any unresolved comments.

B

And if deaths was I can't read it that but.

A

It's right here right, yep, um 332, 332,.

C

Yep all the outstanding comments.

B

Yeah I'd be happy to happy to uh press the merge button. um I think it's pretty much ready to go. There's no, no drama around it. At this point.

A

A

All right was there any um just talk about points to be able to go through for this or uh Chris is good to go just to kind of merge that, on um in the coming days,.

B

Well, I can press the button right now unless there's any um controversy, because it's pretty much just testing the compiler at this point- and you know okay.

D

C

Do it yeah I mean to be more.

E

Fully useful right, like we still have compilation working.

B

Yeah there's a CI Pipeline and it runs the compiler. So this will this will actually just make it. You know run every time, someone submits code, it's going to compile it as another separate job using the dpdk backend. There doesn't run any functional unit tests because you don't have a back end yet.

E

D

C

Doesn't break anything you know, that's what yeah.

E

It doesn't break anything for.

C

E

Okay, that's my first question: thanks and I'm, fine.

B

Oh yeah, that that's the most important question.

C

I mean this is, we are not cutting.

B

C

It's just that this option now begins to exist in the system and pmb2 is still the validation. Yes, yes,.

E

Yeah but I mean like the order, needs to be preserved first, not to break something then to include new stuff. Then we can safely uh move to the new black one.

B

Yeah yeah and Marine that's kind of almost automatic that it runs that check, because what Andy did was put in some if deaths and make some minor code changes to support alternate back in and it runs the same Dash CI pipeline as before. So it's always running bmb2 tests. The pack is going through the pipeline, so yeah.

D

Our regression will continue to be the same as it was before. We want to ensure that okay, we you know, we preserve bmv2, we run the BMV to tabs and everything, but I think you know a couple of things uh Chris.

D

You know as part of this merge after this much as you are mentioning, we need to validate the those containers that we are basically putting into ACR and download to make sure that we're bringing in correctly, which we cannot test it as part of the the pr itself and that's one point and second point is basically I- just wanted to make sure that you know in in within the the CI.

D

We should at least include the compilation of dpdk and, of course the running is not there, but since we are doing the compilation, we do both compilation with bmv2 as well as dbdk, but we run test only on the bmv2 so as to make sure that tomorrow somebody else checks in any code. They don't break the dpdk right. That's what my.

B

Point yeah and if it does break the dpdk at least the the pmb2 will continue to work. Presumably, and we can yeah.

D

Yeah I want to preserve it. Yeah I want to preserve the compilation, because the whole idea here is for us to check in this thing that whatever is you know sitting behind it. If, tomorrow, like, for example, yesterday, we reviewed those tags things right, all the ACL changes in the in the in the in the P4 code yeah. So now that is waiting in the wings to be checked in by Microsoft and once this goes in once this gets merged in they are going to basically bring those changes in there.

D

So they'll make sure that okay, you know both compilations are done, bmv2 as well as dpdk to preserve and not break whatever has been done so moving forward that that if we can ensure the CI catches, this thing that will be great yes,.

B

And that that is what's actually this, this PR will ensure that um it will compile for both targets and um and run tests on the BB2.

B

And if someone checks in something that breaks the dpdk compilation, you'll find it right away and awesome um two two other points you want to make going back one as soon as we accept this merge, it's going to run in the dash Pipeline and um it will.

B

It will publish and pull a new Docker image so we'll find out as soon as we merge this. If there's a problem there- and there could be a little one and I'll just jump on it- sometimes a tag or some typo, some kind of oversight, so I'll just be watching like a hawk and if I have to fix it. I will um and uh the other point is we're not just checking now the dpdk you know put that in capital letters we're also checking the p a architecture.

B

Compilation to me: that's actually more important right, right, bmv2 using V1. Now the code basically running against a p, a architecture and whether it was dpdk or XYZ backend. It's the architecture, we're also verifying and kind of we're sort of I would say elevating this code base to be more aligned with the industry. Mainstream right, yeah, oh I, think that's an important accomplishment. It's not just the dpdk that.

F

D

That is true, good, good point uh Chris. um You know, obviously, as we as we do this you know uh publishing and then pulling the docker image. I I have a quick question on that that we we build these Docker images to to build the um you know BMV to uh separately, and then there is another one Docker image for for doing the dpdk PNA part. However, both of them actually utilize p4c compiler right. So the question here is: do they point to the same?

D

uh uh You know image bring it in from whatever the docker uh registry that they bring in the p4c. We use the same sha um tag or or is it different.

B

uh That's a good question: they're, not the same right now. um I I froze the shop for bmv2 some time ago, so that we don't get surprises. We I don't want someone making a change to p4c. That seems innocuous and suddenly breaks our project right.

B

So it's it's locked into a certain shop and the the dpdk I pulled latest p4c, which I probably should lock that as well, um although that that's probably debatable I just pull latest from from Docker hub yeah I saw that I freeze the shaw for everything else, because I don't want surprises. You know someone side effect right, yeah, yeah. Absolutely that way we have. We have Surety, but.

D

I guess real repeatable repeatable way of actually doing it right. We.

B

D

Do the latest, then you know you.

B

Don't know what's going to break no I learned that a long time ago, the hard way it's not in production, I, don't see the harm in leaving it latest and if uh but I could freeze it. It's just that. It's a maintenance thing. If we want to keep tracking and update since it's there's ongoing work, what do people think? Should we freeze it in which case we'd hold this PR just another day, I'll I'll freeze the shot for that.

F

The latest we4c I should have uh the dpdk support and all the newer PNA um uh constructs Etc that are needed here for Dash, um so um I would say that uh we can, if something is working and then we can freeze that sure and and if there are any new changes at some point in in you know, um as there are new updates that are really necessary, we could update the Sha and try uh running with the latest p4c at that time. uh Otherwise, there's no need to move every single time. Maybe.

B

D

You know, which is which is a good thing, to see that okay, there are a lot of development.

F

D

uh And then you know in ongoing basis right, so there is a lot more activity. However, when you look at the bmv2 and the V1 model right, it's more like you know, not much is happening right, so freezing it over. There, probably you know, uh makes sense, but leaving floating, probably on the on the PNA and dbdk makes sense.

D

Also, but at some point we may want to converge and say: okay, we want to use just one um one uh version of the compiler so as to have some sort of a parity between the two yeah.

B

So yeah we could leave it floating for now, because then.

F

It actually it.

B

Acts like a canary right- in other words it might break our build, but there's no there's no Fallout and we'll get an early warning. We may have to even alert people on those other projects. Hey exactly you broke something. Why yeah exactly so.

F

Maybe all right so I totally agree leaving it to the latest at this point is totally fine um once we have it running and at that point, if we want to when we switch over or we are actively using it, we can increase certain shows and then upgrade as needed.

B

Okay, well then, that's less work for me today works for me as a good discussion, though, because there's pros and cons of allowing things to break okay, great I have to disappear for a few minutes, I'll be back all righty.

A

Hey any conversation there awesome awesome um wanted to possibly shift gears to this pull request. uh Number 308.

E

It's still in progress, we had one review and found a few issues and inbound is not complete yet so I'm waiting for more more guidance from Prince on the inbound.

A

You said uh pertaining to the inbound inbound table.

E

Yeah inbound processing, there is uh something that we both don't don't understand, because service tunnel, as it is described right now, allows to change and early Source IP, and we don't really understand how the packet will in what form the packet will come back to us. What IP it will carry there's a destination if we changed on the way out. If we change Source ID, um we don't know how the returning packet will look like in that case,.

A

So you were pretty much mentioning that the packet it was as if like the packet was leaving and then you change the the destination address. You were saying.

E

Oh so when the packet is leaving the dpu and the outbound Direction, uh there is an option to change the source IP. Now what we don't know, how will the packet look like in that case, when it's coming back? Will it have that modified Source, I uh IP now as destination, or will it have the dpu's virtual IP?

E

That's something we don't know.

A

D

F

There anything there.

D

um Is this not captured in the uh in the agility.

E

uh No, it's not it's just the option to uh have the source IP modified like what how the packet looks like in on the inbound. In that case, it's not captured and that's what friends is trying to um to actually understand from uh from the team. That's using and deploying server Style.

D

Okay, so, but let's make sure that you know we have some either the issue open or whatever be the case um so as to modify the hld as soon as we have some resolution on this. So we don't, you know we have somewhere to. We have somewhere to document this thing. Okay, yeah.

A

I was about to say, um I I didn't I was gonna ask if we had anything added to the actual PR thread, uh but you're you're, just mentioning maybe putting in a uh an issue attached to our our repo that that kind of describes this yeah yeah.

D

uh At least we know that this is an open issue um and somebody is basically is is looking into getting the answers for us, but once the answer comes back, we are not only just going to put this one into saying: okay, let's go ahead, and you know address this one in this PR and modify the P4 code, but also make sure to document individually um so as to capture both documentation, as well as the code.

A

Okay, so being able to understand that if we, if we do I, go to figure out and kind of get an answer back from prints or or whoever, but we also have to um create just another task to update the documentation as well. Exactly.

D

Thank you because.

A

D

Already do this thing yeah, we already do this thing in the Sonic Dash agility that we have, that captures the workings of service tunnel and private link and all of those things right and- and it goes it walks through the scenarios. So this is where we need to put this the update. Okay,.

A

um Who would be able to kind of create that issue attached to our repo and kind of just make sure we update the ACL the hld? Once we have? You know we have our answers for for the inbound processing.

D

I'm volunteering: you can you do that? Please, okay, I got you. I can do.

A

D

A

Course and then, if I, if I be able to create, create that issue, um I'm gonna just show you what kind of draft I have before posting it and we can add any extra information necessary sounds good.

D

A

Okay, awesome awesome um anything else when it comes to this uh we're kind of just at the point, where waiting waiting for a little bit of feedback from Sunday um other than that any other Thomas concerns for this PR.

A

F

We will review it. Okay,.

A

Awesome um all right so taking a look at the rest of these PRS or any other discussion topics. um Anything we wanted to cover brother.

A

Jump back to February, February, 9th.

F

A

All right, so it seems like we were able to get to a good point. There.

A

A

All right so I'm just going to go ahead and take this action item to be able to update our um just our task issues for this and make sure that we can update the hld once we have our PR completed here. But if that is it from folks or no topics at the top of our minds, I think we we can close out about. You know halfway early that if that sounds good with everybody else by me. Yeah all right, awesome, awesome, okay, so I'm going to go ahead and stop recording and.