inet-framework / inet

INET Framework for the OMNeT++ discrete event simulator
https://inet.omnetpp.org
Other
434 stars 486 forks source link

Credit accumulation on closed transmission gates #848

Closed Clunphumb closed 1 year ago

Clunphumb commented 1 year ago

Hello, I am trying to combine Credit-Based Shaping with Time-Aware Shaping and stumbled upon a problem that is hindering me in simulating my network. I noticed that queues, which use the Ieee8021qCreditBasedShaper as transmission selection algorithm, are accumulating credit while their respective transmission gate is closed. This leads to an increase of burstiness in queues as they are able to send much more of their data right after reserved time-slots. Moreover this leads to a shift of the earliest point in time a lower prioritized queue is able to transmit and therefore an increase in delay for this queue. An alternative to the current behavior would be to freeze the current credit of queues whose transmission gate is currently closed as proposed in a note in the Standard IEEE Std 802.1Q-2018 p.201. However, I don't know how to approach this problem myself and would be grateful for some advice on the issue.

levy commented 1 year ago

You should look at the CreditBasedGate module for the implementation of the credit-based shaping algorithm. You could change this module to check if a packet can be pulled from the connected module on its input. If no packet is available then the number credits should not increase.

Clunphumb commented 1 year ago

Hi levy, thank you for your reply. Sadly I wasn't able to implement this solution as I'm not yet familiar with the framework and I didn't completely understand your solution. Am I supposed to check if I can pull from CreditBasedGate or from the successor of the CreditBasedGate? If the ladder is the case i'm unsure on how to get the successor of the successor of the CreditBasedGate to check that. Could you point me in the right direction? On another note, I found that PacketGateBase defines a variable called isOpen_ which for me seems to be something I could maybe use somehow?

Thanks in advance

Clunphumb commented 1 year ago

For anyone interested in this: I think I managed to implement it the way I want it to work by adding a ModuleRef<PeriodicGate> to the CreditBasedGate.h and initialized with periodicGate.reference(outputGate, false);. I also added the Method virtual bool isTransmissionGateOpen() const {return periodicGate->isOpen();} and used it in updateCurrentCreditGainRate() to set currentCreditGainRate = 0. I don't know much about C++ or OMNeT or INET but this approach seems to be working.

PhilippComet commented 1 year ago

Hi, unfortunately i have the same issue. @Clunphumb Thank you for the approach, but can you please explain a bit more detailed your modifications? Where did you add periodicGate.reference(outputGate, false);?

Additionally here a visualization of the problem of credit accumulation during a closed TAS Gate. TAS Gate closes here in this image at t=0.004

credit accumulation

Clunphumb commented 1 year ago

Hi @PhilippComet, I'm happy to provide some more insight. To be more precise I added:

Hope this works for you.

PhilippComet commented 1 year ago

Hi @Clunphumb,

thank you for your fast answer. It became clearer to me. But this actually did not work for me directly.

As you can see in the previous and following image, there could be also the scenario that a packet is not dequeued even when 1.) the gate is still open and 2.) the credit value is positive (i still don't know why). Then the gate would close with one packet in the queue and the packet is dequeued only when the gate is reopened again. And until this point in time the credit will rise (see image in my previous comment).

credit accumulation problem

I think that there has to be additionally a modification in updateCurrentCredit()

void CreditBasedGate::updateCurrentCredit()
{
    if (isTransmitting)
        currentCredit = lastCurrentCreditEmitted + currentCreditGainRate * (simTime() - lastCurrentCreditEmittedTime).dbl();
    else
        currentCredit = lastCurrentCreditEmitted;
    currentCredit = std::max(minCredit, std::min(maxCredit, currentCredit));
}

then credit evolution looks more correct

credit paused accumulation correct

Clunphumb commented 1 year ago

Hi @PhilippComet,

thanks for your contribution to the solution.

I would claim that the observed behavior before you made further changes is caused by the implementation of the PeriodicGate. The function canPacketFlowThrough checks, whether a packet can be transmitted before the gate is closed. As is_open only checks whether the gate is opened or not, the accumulation still continued eventhough no packet could be transmitted.

Did you check if your solution works for multiple queues with the CBS attached? If the credit is only updated when the gate is transmitting, this would mean, that while another higher-prioritized CBS queue is transmitting and supressing the lower prioritized queue, the credit wouldn't get updated accurately. However this is just a guess on how this could cause problems as well.

I think an approach using canPacketFlowThrough of the PeriodicGate would give the correct solution but I haven't tried yet. I will look into this again in the coming days.

Clunphumb commented 1 year ago

Hi @PhilippComet,

I was already able to take a deeper look into the issue and the corresponding Standard IEEE Std 802.1Q-2018. This lead to two insights for me:

  1. It is stated in Note 1 on page 204: "It is assumed that the implementation has knowledge of the transmission overheads that are involved transmitting a frame on a given Port and can therefore determine how long the transmission of a frame will take. [...] It is desirable that the schedule for such traffic is designed to accommodate the intended pattern of transmission without overrunning the next gate-close event for the traffic classes concerned." This is directly reflected in the behavior of the Periodic Gate where Packets, that can't be transmitted wholly until the Gate Close Event, aren't sent in this period.

  2. On page 201 "idleSlope" is defined as "The rate of change of credit, in bits per second, when the value of credit is increasing i.e., while transmit is FALSE and the transmission gate for the queue is open [8.6.8.4])".

Combining those two findings I would argue, that even though a CBS can't send, due to the "smart" behavior of the Periodic Gate, the credit isn't frozen yet and has to accumulated until the gate is closed properly.

I implemented this behavior and got the following result:

state_credit_queue_plot

As you can see, when another packet is queued shortly before the gate is closed the credit is increased as long as it remained open.

The implementation however has been increasingly difficult but if you are be interested in this solution I would give it a try and explain my approach or upload the code.

levy commented 1 year ago

This is an interesting discussion. It would be great if you could cook something up and submit a PR. If there are multiple different valid behaviours then preferably the change should include a new parameter to select among them.

PhilippComet commented 1 year ago

Hi @Clunphumb, thank you very much for having a deeper look and effort so far. As you suggested, I will try to run an example with two traffic classes to see the impact of competing two shapers in credit evolution.

Is this also the case at t=0.0007s that a packet is in the queue but transmission not allowed because of another flow (another priority) is in transmission?

I would be very interested in having an insight how you solved this to get to the last plot you uploaded.

Dependant on the output of canPacketFlowThrough(), i would also like to implement with the ability to switch between your example (as described in the standard) and freezing the credit evolution already during the "guardband" time.

This was also investigated e.g. in: H. Daigmorte and M. Boyer, “Impact on credit freeze before gate closing in CBS and GCL integration into TSN,” in Proc. Int. Conf. Real-Time Netw. Syst., 2019, pp. 80–89.

Clunphumb commented 1 year ago

Hi @PhilippComet,

I pushed my code on a fork I created. You should be able to see it in my profile. I haven't implemented an option to change between the settings yet but I will probably do it later on. Moreover my code isn't very sophisticated but I guess it does the job for now.

The paper you mentioned is very interesting and proposed a problem I wasn't quite aware of. I'm unsure now of what the "right" way would be to handle credit accumulation now as the standard doesn't seem to propose a solution for this problem. On the other hand since for this problem to occur, the queues of all gates would have to be filled constantly, I would guess that this won't be such a grave problem for real-world applications.

For t=0.0007s you are right, at this time another queue had priority and received service.

PhilippComet commented 1 year ago

Hi @Clunphumb, thank you very much for your effort. Coincidentally, I was working parallely with kind of the the same approach using the gate state change signal as trigger, but was stuck at one point.

Yes, the current standart does not a provide a guideline how to handle the credit evolution during this pre-closing time. In another work I read, that traffic classes with higher priority (and higher idleslope) could gain higher credit in this time period which would lead to bursty behavior of higher priority classes.

I'm still not sure how the credit evolution behaves with this modification when multiple Credit-Based Shapers are assigned to different priorities. I will test an example tomorrow.

PhilippComet commented 1 year ago

Hi @Clunphumb,

it seems that there is occuring new unusual behaviour with the new modification. I adopted you changes. There could be a wrong trigger that freezes credit of the other flow when one of both curves hitting the zero. In this example i only used two flows with two different priorities sharing the same TAS time-window. Did you observe the same?

redit evo mod multi traffic problem 3

Clunphumb commented 1 year ago

Hi @PhilippComet,

thanks for your feedback. I didn't notice this problem yet so thank your very much for testing!

It was caused by the handling of the gateStateChangedSignal which is also used by the CreditBasedGates additionally to the PeriodicGates. I didn't think about adding a check where the signal is coming from before. I updated my code so this should be fixed for now.

PhilippComet commented 1 year ago

Perfect, now it works. Thanks for your effort!

Clunphumb commented 1 year ago

Hey, I'm glad it's finally working as desired. I created a pull request and noticed some more smaller mistakes while restructuring and refactoring my code regarding calculation of the credit increase during pre-close time. I think this should be fixed now and hopefully be merged into INET in the future

PhilippComet commented 1 year ago

Hi @Clunphumb,

Since some time I have a problem with a certain configuration and the frozen credit modification we talked about above. What you can see in the picture are two queues - one of them equipped with a CBS for AVB Traffic and another one with higher priority with Scheduled Traffic (TT). Both are connected with transmission gates of the TAS which is controlled by a custom Gate Control List. (visible in blue and orange upper graphs)

grafik

For the Scheduled Traffic everything works fine (you can see the gate switching in orange and queue length in the lowest violet curve). But after around 2ms the credit will start to infinitely accumulate and isOpen_ of the CBS will never get "true" again even though the credit is positive and the implicit guardband is not active (aka canPacketFlowThrough()). I'm investigating already some time on this issue. Do you have an idea what could be the problem?

grafik

here a bit more macroscopic view:

grafik

levy commented 1 year ago

Can I get the example simulation that you are using to validate the CBS/TAS behavior?

I think this issue should be fixed before the INET 4.5 release. Could you also please take a look at #852?

Clunphumb commented 1 year ago

Hi @PhilippComet,

I've tried to investigate the issue. However, I was unable to reproduce the problem, which severely decreased my chances of finding the cause for this. I built the same setup with two queues and tried several combinations of different configurations of time slots and packet production interval, but I didn't get to a point where the CBS won't be open ever again. To investigate further I would have to take a look at your configuration.

PhilippComet commented 1 year ago

Hello @Clunphumb and @levy ,

I am sorry that I am replying only after two weeks, but I had a serious family emergency that I needed to take care of.

I have made sure that the problem has not by chance already been solved in the meantime by pulling the current INET version, but unfortunately the problem remains.

What is the best way to provide you the network configuration files with which I generated the above shown problem? Via e-mail?

levy commented 1 year ago

I hope your family is ok.

The best would be to upload the network and ini files to GitHub on the original issue page.

Best regards, levy

On Sun, 19 Mar 2023, 00:47 PhilippComet, @.***> wrote:

Hello @Clunphumb https://github.com/Clunphumb and @levy https://github.com/levy ,

I am sorry that I am replying only after two weeks, but I had a serious family emergency that I needed to take care of.

I have made sure that the problem has not by chance already been solved in the meantime by pulling the current INET version, but unfortunately the problem remains.

What is the best way to provide you the network configuration files with which I generated the above shown problem? Via e-mail?

— Reply to this email directly, view it on GitHub https://github.com/inet-framework/inet/issues/848#issuecomment-1475034389, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA3OTJAX32WKMRSHUWAXCDW4ZCQJANCNFSM6AAAAAATSDH7GA . You are receiving this because you were mentioned.Message ID: @.***>

levy commented 1 year ago

I pushed everything to master to make it easier for you to test the current implementation.

Using the current INET master branch, the CBS gate doesn't increase the number of credits while the TAS gate is closed. Additionally, the CBS gate doesn't increase the number of credits while the TAS gate is in implicit guard band. There's a new statistic for the guard band state that you may want to include in your charts.

Could you please verify if the current master branch solves this issue? If not, could you please send the NED/INI files here that demonstrate the problem?

Thanks!

PhilippComet commented 1 year ago

CBS_TAS.zip

Thank you a lot for updating the code with the new features. Ok, I re-tested the latest version. In the following the same infinite growing behaviour of the credit value occured again. As you can see this is strange, because everything works correctly for the first 2 milliseconds and then something fails. As you can see the guardband for AVB Traffic in green only is functional until the point of failure. The location of the signals you can see in the graph is the egress port 4 of Switch 1 in the network:

SW1_eth4_with_deactivated_GB_for_ST_annotated

more zoomed in: (0ms-2.5ms)

SW1_eth4_with_deactivated_GB_for_ST_zoom

Some additional side facts to the network:

Please let me know, when you need more information (quick Zoom call, etc.) or something is unclear.

Thank you for your support!

levy commented 1 year ago

Thanks for the example and the detailed description! Finally, I could reproduce the problem and find out what causes it.

The reason for the issue was that the change timer was scheduled for the exact same moment when a packet has arrived in the queue. This in turn caused the change timer to be canceled and not scheduled ever again. The result is that the number of credits to increase indefinitely.

I created a fix in the topic/cbs branch. This change makes sure that the gate is open above transmit limit and closed below no matter in what order state changes happen. Also, the change timer is only scheduled for the future, and not for the current simulation time, because credit doesn't change in zero amount of time.

With this change your network produces the following chart for the same CBS traffic:

image

Could you please verify if this fixes your issue?

Additionally, this change keeps the network level fingerprints of existing example simulations, though it changes the detailed fingerprints. I think this is ok.

PhilippComet commented 1 year ago

Thank you very much for solving the issue and the explanation of the reason. I could reproduce it and now it works correctly for the "frozen during guardband" version.

But when I checked the same "non-frozen during guardband" configuarion with .accumulateCreditInGuardBand = true parameter in the .ini file, I saw that the credit value should be set back to zero, when the queue is empty (according to the IEEE standard):

grafik

levy commented 1 year ago

Thanks for the fast response! I could reproduce the above issue: the number of credits is not set to zero when the queue becomes empty. I'm looking into what causes this, because it's supposed to be implemented.

levy commented 1 year ago

Could you please check the topic/cbs branch if it works as you expect?

This branch passes the numeric CBS validation test and keeps the network level fingerprints of the relevant examples.

PhilippComet commented 1 year ago

Thank you very much for the update. I pulled the topic/cbs branch again. Now the credit evolution curve looks correctly. I don't know what cased the issue in my last post, but now it works. Thanks again!

non-frozen credit during guard band: grafik

frozen credit during guard band: grafik

PhilippComet commented 1 year ago

I have found another feauture, that might be interesting to be added to the current CBS implementation to match the IEEE 802.1Q-2018 standart.

In the CBS chapter 8.6.8.2 d.) on page 201 there is a note that when using CBS +TAS the idleslope has to be increased by a certain factor that compensates the closed gate intervals by a weighting factor to keep the effective datarate that the user assignes to traffic shaped by the CBS.

In the current implementation for example if the user assignes 50Mbit/s and the AVB gate is closed 50% of the time (due to TAS operation), the resulting effective data rate would be 25Mbit/s. This could be solved by adding a weighting factor to the idleCreditGainRate.

idleCreditGainRateCompensated = idleCreditGainRate * (OperCycleTime / GateOpenTime)

Where OperCycleTime is the hyperperiod of the GCL and GateOpenTime the respective overall gate open duration per GCL cycle

grafik

levy commented 1 year ago

Thanks for verifying. The above changes have been merged into INET master.

I also created a new issue for your last comment.