StreamRedundancyConfigurator: A few bugs and a patch that might help

[xiaoyuyaa555]

Hi INET team!

While working on a TSN simulation project using StreamRedundancyConfigurator, I ran into a few issues that made redundant stream configuration fail or behave unexpectedly. After digging into the code, I found some potential problems and made a patch that seems to fix them in our case — just wanted to share with you and maybe get your thoughts!

For simple topologies and traffic conditions, there's no problem using this code to configure flow redundancy scheduling, and its basic logic is correct. However, my current traffic situation is quite complex, and there will be abnormal filtering behavior of the merger. Specifically, for some flows, when the two data packets generated by frame replication reach the frame elimination node, one should be discarded and the other retained, but the merger will directly filter out both data packets.

In addition, I've also found that the PCP of data packets will be set twice, and the values are even different, which leads to incorrect PCP settings for some data packets. I think this might be a problem with flow identification, resulting in repeated settings.

Upon inspection, the following problem was found in the previous code:

• VLAN ID Conflicts

auto jt = nextVlanIds.emplace(std::pair<std::string, std::string>{networkNodeName, destinationAddress}, 0);
int vlanId = jt.first->second++;
Issue: VLAN ID assignment is only based on the source node and destination address, not uniquely bound to each stream. This may lead to ID collisions when multiple flows share the same source and destination pair.

• Inconsistent State Between Upstream and Downstream Nodes

auto vlanId = assignedVlanIds[{senderNetworkNodeName, networkNodeName, destinationAddress, streamName}];

Issue: The code assumes that upstream configuration has already been performed, which is not always the case. If the initialization sequence is disrupted, downstream nodes may access uninitialized VLAN IDs, causing runtime errors.

• Missing Interface and Link Validations

if (interfaceName.empty())
    interface = findLinkOut(node, receiverNetworkNodeName.c_str())->sourceInterface;
Issue: The code does not validate whether findLinkOut() returns a null pointer. In incomplete topologies, this may lead to crashes.

 Incomplete Configuration Cleanup
void StreamRedundancyConfigurator::clearConfiguration() {
    streams.clear();
    nextVlanIds.clear();
    assignedVlanIds.clear();
    if (topology != nullptr)
        topology->clear();
}

Issue: The configurator clears its internal state but does not notify affected modules (e.g., redundancy modules in network nodes), leading to stale configuration data and inconsistent runtime states.

I made the following changes:

1. Fix the abnormal filtering issue of the merger

   • Avoid merger confusion by ensuring the uniqueness of input stream names.
   • Use std::set<std::string> uniqueInputStreams to ensure that there are no duplicate input stream names.
   • Add debugging information to track the merger configuration.

2. Solve the problem of repeated PCP settings

   • Use StreamFlowContext to track the PCP configuration status of each stream.
   • Prevent repeated PCP configuration on the same node through the configuredNodes collection.
   • Manage PCP values uniformly during the encoding phase.

3. Global VLAN ID management

   • Implement global VLAN ID allocation to avoid conflicts.
   • Maintain independent VLAN ID usage records for each destination address.

4. Enhance configuration validation

   • Add link existence checks.
   • Validate the effectiveness of VLAN IDs.
   • Prevent repeated configuration of the same flow policy.

I proved that the change works by simulation testing. If my changes are valid, can I make Pull requests to contribute to the INET? This is the first time I've done something like this.

Finally in the attachment I have uploaded the modified .h and .cc files.

StreamRedundancyConfigurator.cc.txt
StreamRedundancyConfigurator.h.txt

[levy]

Hey, thanks for taking the time to write this down!

Actually, it would be easier for me to understand your reasoning and the related suggested changes in the code if you would submit a pull request. Please try to separate the changes into several patches. If your analysis is correct, then I'm happy to add your changes to the configurator in your name.

Please submit a PR!

[levy]

Please note that the vlan ID may not be the same along the path for a given stream. The vlan ID may be different on each link for the same stream and this is not a problem at all. Each node has its own member stream encoding and decoding parameters and allocating the same vlan ID along the whole path would be more difficult than allocating the vlan ID for each step separately.

[xiaoyuyaa555]

请注意，给定流的路径上的 vlan ID 可能不同。同一流的每个链路上的 vlan ID 可能不同，这根本不是问题。每个节点都有自己的成员流编码和解码参数，在整个路径上分配相同的 vlan ID 比为每个步骤单独分配 vlan ID 更困难。

Thank you for your reply. I think what you said makes sense. I didn’t take this point into consideration when I was designing. Next, I may need to spend some time organizing and rewriting my revisions.