Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-action parameter support in dash libsai #654

Open
jimmyzhai opened this issue Dec 20, 2024 · 0 comments
Open

Per-action parameter support in dash libsai #654

jimmyzhai opened this issue Dec 20, 2024 · 0 comments
Assignees

Comments

@jimmyzhai
Copy link
Collaborator

Currently API create will set all attributes to action parameters of each action of p4 table. If one p4 table has multiple actions and their parameters have overlap, API create may have unexpected behavior.

To check table routing as an example:

    @SaiTable[name = "outbound_routing", api = "dash_outbound_routing"]
    table routing {
        key = {
            meta.eni_data.outbound_routing_group_data.outbound_routing_group_id : exact @SaiVal[type="sai_object_id_t"];
            meta.is_overlay_ip_v6 : exact @SaiVal[name = "destination_is_v6"];
            meta.dst_ip_addr : lpm @SaiVal[name = "destination"];
        }

        actions = {
            route_vnet(hdr, meta); /* for expressroute - ecmp of overlay */
            route_vnet_direct(hdr, meta);
            route_direct(hdr, meta);
            route_service_tunnel(hdr, meta);
            drop(meta);
        }
        size = TABLE_ROUTING_SIZE;
        const default_action = drop(meta);

        ATTACH_TABLE_COUNTER(routing_counter)
    }

// Routing Type - vnet:
// - Continue to look up in VNET mapping stage with the destination VNET ID.
// - No routing action will be populated in this routing type.
action route_vnet(
    inout headers_t hdr,
    inout metadata_t meta,
    @SaiVal[type="sai_object_id_t"] bit<16> dst_vnet_id,
    @SaiVal[type="sai_object_id_t"] bit<16> dash_tunnel_id,
    bit<32> meter_class_or,
    @SaiVal[default_value="4294967295"] bit<32> meter_class_and,
    dash_routing_actions_t routing_actions_disabled_in_flow_resimulation)
{
    meta.dash_tunnel_id = dash_tunnel_id;

    meta.target_stage = dash_pipeline_stage_t.OUTBOUND_MAPPING;
    meta.dst_vnet_id = dst_vnet_id;
    set_meter_attrs(meta, meter_class_or, meter_class_and);
}

// Routing Type - vnet_direct:
// - Forward with overrided destination overlay IP.
// - No routing action will be populated in this routing type.
action route_vnet_direct(
    inout headers_t hdr,
    inout metadata_t meta,
    bit<16> dst_vnet_id,
    @SaiVal[type="sai_object_id_t"] bit<16> dash_tunnel_id,
    bit<1> overlay_ip_is_v6,
    @SaiVal[type="sai_ip_address_t"]
    IPv4ORv6Address overlay_ip,
    bit<32> meter_class_or,
    @SaiVal[default_value="4294967295"] bit<32> meter_class_and,
    dash_routing_actions_t routing_actions_disabled_in_flow_resimulation)
{
    meta.dash_tunnel_id = dash_tunnel_id;

    meta.target_stage = dash_pipeline_stage_t.OUTBOUND_MAPPING;
    meta.dst_vnet_id = dst_vnet_id;
    meta.lkup_dst_ip_addr = overlay_ip;
    meta.is_lkup_dst_ip_v6 = overlay_ip_is_v6;
    set_meter_attrs(meta, meter_class_or, meter_class_and);
}

Check the generated code of API dash_sai_create_outbound_routing_entry:

    for (uint32_t i = 0; i < attr_count; i++)
    {
        auto *md = sai_metadata_get_attr_metadata((sai_object_type_t)SAI_OBJECT_TYPE_OUTBOUND_ROUTING_ENTRY, attr_list[i].id);

        const char* attrName = md ? md->attridname : "unknown";

        switch(attr_list[i].id)
        {
            case SAI_OUTBOUND_ROUTING_ENTRY_ATTR_DST_VNET_ID:
            {
                auto param = action->add_params();
                param->set_param_id(1);
                u16SetVal(attr_list[i].value, param, 16);
                //matchedParams++;
                break;
            }
            case SAI_OUTBOUND_ROUTING_ENTRY_ATTR_DASH_TUNNEL_ID:
            {
                auto param = action->add_params();
                param->set_param_id(2);
                u16SetVal(attr_list[i].value, param, 16);
                //matchedParams++;
                break;
            }
            case SAI_OUTBOUND_ROUTING_ENTRY_ATTR_METER_CLASS_OR:
            {
                auto param = action->add_params();
                param->set_param_id(3);
                u32SetVal(attr_list[i].value, param, 32);
                //matchedParams++;
                break;
            }
            case SAI_OUTBOUND_ROUTING_ENTRY_ATTR_METER_CLASS_AND:
            {
                auto param = action->add_params();
                param->set_param_id(4);
                u32SetVal(attr_list[i].value, param, 32);
                //matchedParams++;
                break;
            }
            case SAI_OUTBOUND_ROUTING_ENTRY_ATTR_ROUTING_ACTIONS_DISABLED_IN_FLOW_RESIMULATION:
            {
                auto param = action->add_params();
                param->set_param_id(5);
                u32SetVal(attr_list[i].value, param, 32);
                //matchedParams++;
                break;
            }
            case SAI_OUTBOUND_ROUTING_ENTRY_ATTR_OVERLAY_IP:
            {
                auto param = action->add_params();
                param->set_param_id(4);
                ipaddrSetVal(attr_list[i].value, param, 128);
                //matchedParams++;
                {
                    // set ip_is_v6_field_id field
                    auto param2 = action->add_params();
                    param2->set_param_id(3);
                    booldataSetVal((attr_list[i].value.ipaddr.addr_family == SAI_IP_ADDR_FAMILY_IPV4) ? 0 : 1, param2, 1);
                    //matchedParams++;
                }
                break;
            }

The attr METER_CLASS_AND and OVERLAY_IP both apply to one parameter (position id 4). It's a bug. Need to set per-action parameter.

@jimmyzhai jimmyzhai self-assigned this Dec 20, 2024
r12f pushed a commit that referenced this issue Jan 8, 2025
Following #651, refactor APIs
create/remove for easier read and maintenance:

1. Use common code `DashSai::create` and `DashSai::remove` to implement
them for all SAI objects/entries
2. Fix issue #436, now only 1
SAI object is created for multiple bmv2 objects (each bmv2 object as a
p4 table entry in p4 table for each stage).
3. Fix issue #654

The generated sample code of APIs create/remove attribute is as below:

- SAI object objectID

![image](https://github.com/user-attachments/assets/1f300b7a-7750-43df-89e2-71570801cd06)

- SAI object entry

![image](https://github.com/user-attachments/assets/d16a3bcc-0a41-4fd1-a23a-6f6663761a67)

For the case of acl rule object, one acl rule is inserted into multiple
p4 tables in different stages. These p4 stage tables are same and the
only difference is `table id` and `table action id` in table entry,
captured in struct P4MetaSiblingTable:

```
    struct P4MetaSiblingTable
    {
        uint32_t id;
        // action enum id -> p4 action id
        std::map<uint32_t, uint32_t> actions;
    };
```
The generated code of sibling tables for acl rule is as below:

![image](https://github.com/user-attachments/assets/4dbf0725-1ca5-488f-bf08-62b032c2f66a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant