ngsdn-tutorial icon indicating copy to clipboard operation
ngsdn-tutorial copied to clipboard

L2BridgingComponent.java ternary match problem

Open makrofag1 opened this issue 3 years ago • 6 comments

Hi, I have observed this ONOS warn in onos logs: Unable to INSERT table entry on device:leaf2: OTHER_ERROR INVALID_ARGUMENT Invalid representation of 'don't care' ternary match, omit match field instead of using 0 mask (:3) [PiTableEntry{tableId=IngressPipeImpl.l2_ternary_table, matchKey={hdr.ethernet.dst_addr=0x0&&&0x0}, tableAction=IngressPipeImpl.set_multicast_group(gid=0xff), priority=11, timeout=PERMANENT}]

This warn lead me to L2BridgingComponent.java file where we can find criterion that should match any previously unmatched packet (lines 272 - 277). Could you explain me please what is the proper representation of the rule that matches anything? I use onos 2.2.2 and mentioned rule stays in pending add. @bocon13 @ccascone

makrofag1 avatar Dec 17 '21 18:12 makrofag1

The error message seems suggesting that we should omit the entire match field instead of using wildcard mask. Have you tried removing the entire match field on hdr.ethernet.dst?

charlesmcchan avatar Dec 17 '21 20:12 charlesmcchan

I have removed hdr.ethernet.dst, so the criterion looks like this: final PiCriterion unmatchedTrafficCriterion = PiCriterion.builder() .matchTernary( PiMatchFieldId.of(""), MacAddress.valueOf("00:00:00:00:00:00").toBytes(), MacAddress.valueOf("00:00:00:00:00:00").toBytes()) .build();

In the result I have received this exception in onos logs: Exception in thread "pool-13-thread-3" java.lang.IllegalArgumentException: Name cannot be empty at com.google.common.base.Preconditions.checkArgument(Preconditions.java:134) at org.onosproject.net.pi.model.PiMatchFieldId.of(PiMatchFieldId.java:43) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.insertUnmatchedBridgingFlowRule(L2BridgingComponent.java:274) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.setUpDevice(L2BridgingComponent.java:158) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.lambda$setUpAllDevices$2(L2BridgingComponent.java:496) at com.google.common.collect.Iterables$4.lambda$forEach$0(Iterables.java:584) at java.base/java.util.concurrent.ConcurrentHashMap$ValuesView.forEach(ConcurrentHashMap.java:4772) at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1085) at com.google.common.collect.Iterables$4.forEach(Iterables.java:581) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.setUpAllDevices(L2BridgingComponent.java:493) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:829)

There is no way to omit entire object of PiMatchFieldID.of() because in PiCriterion which is used in L2BridingComponent.java all versions of matchTernary() function require 3 arguments. I really don't understand why it doesn't work despite it is a tutorial.

makrofag1 avatar Dec 17 '21 21:12 makrofag1

I was suggesting skipping the entire ethernet dst match field. Something like this:

final PiCriterion unmatchedTrafficCriterion = PiCriterion.builder().build();

This should theoretically create a "catch all" match

charlesmcchan avatar Dec 17 '21 22:12 charlesmcchan

I checked your suggestion but I get: Exception in thread "pool-13-thread-3" java.lang.IllegalArgumentException: Cannot build PI criterion with 0 field matches at com.google.common.base.Preconditions.checkArgument(Preconditions.java:134) at org.onosproject.net.flow.criteria.PiCriterion$Builder.build(PiCriterion.java:334) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.insertUnmatchedBridgingFlowRule(L2BridgingComponent.java:272) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.setUpDevice(L2BridgingComponent.java:158) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.lambda$setUpAllDevices$2(L2BridgingComponent.java:491) at com.google.common.collect.Iterables$4.lambda$forEach$0(Iterables.java:584) at java.base/java.util.concurrent.ConcurrentHashMap$ValuesView.forEach(ConcurrentHashMap.java:4772) at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1085) at com.google.common.collect.Iterables$4.forEach(Iterables.java:581) at org.onosproject.ngsdn.tutorial.L2BridgingComponent.setUpAllDevices(L2BridgingComponent.java:488) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)

makrofag1 avatar Dec 18 '21 00:12 makrofag1

Hmm... alright, looks like it doesn't like it either. Will need to take a closer look to figure out why the original PiCriterion get rejected.

charlesmcchan avatar Dec 18 '21 00:12 charlesmcchan

Hi @makrofag1, you are describing the case where an app wants to modify a table's default action, which is equivalent to a flow rule with all match fields set to "don't care". To create such a flow rule, you simply need to omit adding a selector when invoking the flow rule builder. Unfortunately, in this tutorial we use the utility method buildFlowRule which always adds a selector: https://github.com/opennetworkinglab/ngsdn-tutorial/blob/62705c3cb64fa7c731b5255f5fc793f293ce0665/app/src/main/java/org/onosproject/ngsdn/tutorial/common/Utils.java#L110

You could solve this by creating a different utility method that looks like this:

public static FlowRule buildFlowRuleDefaultAction(DeviceId switchId, ApplicationId appId,
                                                  String tableId, PiTableAction piAction) {
        return DefaultFlowRule.builder()
                .forDevice(switchId)
                .forTable(PiTableId.of(tableId))
                .fromApp(appId)
                .withPriority(DEFAULT_FLOW_RULE_PRIORITY)
                .makePermanent(
                .withTreatment(DefaultTrafficTreatment.builder()
                                       .piTableAction(piAction).build())
                .build();
    }

ccascone avatar Dec 18 '21 00:12 ccascone