From b1881ba7bf8554600eae9a56858339b180bd56ce Mon Sep 17 00:00:00 2001 From: "Georgios P. Katsikas" Date: Wed, 22 Jul 2026 18:20:03 +0300 Subject: [PATCH] fix: p4 driver insert/delete ops. and GET entries in JSON --- src/device/service/drivers/p4/p4_common.py | 4 +- src/device/service/drivers/p4/p4_driver.py | 23 +- src/device/service/drivers/p4/p4_manager.py | 317 ++++++++++++++------ 3 files changed, 242 insertions(+), 102 deletions(-) diff --git a/src/device/service/drivers/p4/p4_common.py b/src/device/service/drivers/p4/p4_common.py index 80255d3fa..0c87b2aab 100644 --- a/src/device/service/drivers/p4/p4_common.py +++ b/src/device/service/drivers/p4/p4_common.py @@ -339,7 +339,7 @@ def parse_action_parameters_from_json(resource): if not resource or ("action-params" not in resource): LOGGER.warning( "JSON entry misses 'action-params' list of attributes") - return None + return {} chk_type("action-params", resource["action-params"], list) action_name = parse_resource_string_from_json(resource, "action-name") @@ -368,7 +368,7 @@ def parse_replicas_from_json(resource): if not resource or ("replicas" not in resource): LOGGER.warning( "JSON entry misses 'replicas' list of attributes") - return None + return [] chk_type("replicas", resource["replicas"], list) replicas = {} diff --git a/src/device/service/drivers/p4/p4_driver.py b/src/device/service/drivers/p4/p4_driver.py index f631e52f0..a7a7605d6 100644 --- a/src/device/service/drivers/p4/p4_driver.py +++ b/src/device/service/drivers/p4/p4_driver.py @@ -35,7 +35,7 @@ from .p4_manager import P4Manager, \ KEY_TABLE, KEY_ACTION, KEY_ACTION_PROFILE, \ KEY_COUNTER, KEY_DIR_COUNTER, KEY_METER, KEY_DIR_METER,\ KEY_CTL_PKT_METADATA, KEY_DIGEST, KEY_CLONE_SESSION,\ - KEY_ENDPOINT + KEY_MULTICAST_GROUP, KEY_ENDPOINT from .p4_client import WriteOperation try: @@ -482,6 +482,19 @@ class P4Driver(_Driver): ap_name) if ap_entries: entries.append(ap_entries) + + ap_group_entries = self.__manager.action_prof_group_entries_to_json( + ap_name) + if ap_group_entries: + entries.append(ap_group_entries) + elif KEY_MULTICAST_GROUP == resource_key: + mc_entries = self.__manager.multicast_group_entries_to_json() + if mc_entries: + entries.extend(mc_entries) # note: returns a list already, not a dict + elif KEY_CLONE_SESSION == resource_key: + cs_entries = self.__manager.clone_session_entries_to_json() + if cs_entries: + entries.extend(cs_entries) # note: returns a list already, not a dict elif KEY_ACTION == resource_key: # To be implemented or deprecated pass @@ -671,11 +684,15 @@ class P4Driver(_Driver): elif KEY_ACTION_PROFILE in resource_key: self.__manager.action_prof_member_entry_operation_from_json( resource_value, operation) - self.__manager.action_prof_group_entry_operation_from_json( - resource_value, operation) + # TODO: See how action profile groups can be incorporated + # self.__manager.action_prof_group_entry_operation_from_json( + # resource_value, operation) elif KEY_CLONE_SESSION in resource_key: self.__manager.clone_session_entry_operation_from_json( resource_value, operation) + elif KEY_MULTICAST_GROUP in resource_key: + self.__manager.multicast_group_entry_operation_from_json( + resource_value, operation) elif KEY_CTL_PKT_METADATA in resource_key: msg = f"{resource_key.capitalize()} is not a " \ f"configurable resource" diff --git a/src/device/service/drivers/p4/p4_manager.py b/src/device/service/drivers/p4/p4_manager.py index ae77c41a3..c221bcc11 100644 --- a/src/device/service/drivers/p4/p4_manager.py +++ b/src/device/service/drivers/p4/p4_manager.py @@ -72,6 +72,7 @@ KEY_DIGEST = "digest" # Extra resource keys KEY_CLONE_SESSION = "clone_session" +KEY_MULTICAST_GROUP = "multicast_group" KEY_ENDPOINT = "endpoint" @@ -619,34 +620,40 @@ class P4Manager: continue table_res["table-name"] = table_name + table_res["entries"] = [] for ent in entries: entry_match_field = "\n".join(ent.match.fields()) entry_match_type = match_type_to_str( ent.match.match_type(entry_match_field)) - table_res["id"] = ent.id - table_res["match-fields"] = [] + entry_res = { + "id": ent.id, + "match-fields": [], + "actions": [ + { + "action-id": ent.action.id(), + "action": ent.action.alias() + } + ], + "priority": ent.priority, + "is-default": ent.is_default, + "idle-timeout": ent.idle_timeout_ns + } + for match_field in ent.match.fields(): - table_res["match-fields"].append( + entry_res["match-fields"].append( { "match-field": match_field, "match-value": ent.match.value(match_field), "match-type": entry_match_type } ) - table_res["actions"] = [] - table_res["actions"].append( - { - "action-id": ent.action.id(), - "action": ent.action.alias() - } - ) - table_res["priority"] = ent.priority - table_res["is-default"] = ent.is_default - table_res["idle-timeout"] = ent.idle_timeout_ns + if ent.metadata: - table_res["metadata"] = ent.metadata + entry_res["metadata"] = ent.metadata + + table_res["entries"].append(entry_res) return table_res @@ -700,6 +707,7 @@ class P4Manager: action_name = parse_resource_string_from_json( json_resource, "action-name") action_params = parse_action_parameters_from_json(json_resource) + member_id = parse_resource_integer_from_json(json_resource, "member-id") priority = parse_resource_integer_from_json(json_resource, "priority") metadata = parse_resource_bytes_from_json(json_resource, "metadata") @@ -710,6 +718,7 @@ class P4Manager: match_map=match_map, action_name=action_name, action_params=action_params, + member_id=member_id, priority=priority, metadata=metadata if metadata else None ) @@ -720,12 +729,13 @@ class P4Manager: match_map=match_map, action_name=action_name, action_params=action_params, + member_id=member_id, priority=priority ) return None def insert_table_entry_exact(self, - table_name, match_map, action_name, action_params, metadata, + table_name, match_map, action_name, action_params, member_id, metadata, cnt_pkt=-1, cnt_byte=-1): """ Insert an entry into an exact match table. @@ -740,9 +750,23 @@ class P4Manager: :return: inserted entry """ assert match_map, "Table entry without match operations is not accepted" - assert action_name, "Table entry without action is not accepted" - table_entry = TableEntry(self.local_client, table_name)(action=action_name) + LOGGER.info(f" Table name: {table_name}") + LOGGER.info(f" Table type: exact") + LOGGER.info(f"Action Name: {action_name}") + LOGGER.info(f" Match: {match_map}") + LOGGER.info(f" Member ID: {member_id}") + LOGGER.info(f" Action: {action_params}") + + table_entry = None + if action_name: + table_entry = TableEntry(self.local_client, table_name)(action=action_name) + else: + LOGGER.warning(f"Table {table_name} with no action name") + table_entry = TableEntry(self.local_client, table_name) + + if member_id > 0: + table_entry.member_id = member_id for match_k, match_v in match_map.items(): table_entry.match[match_k] = match_v @@ -763,9 +787,13 @@ class P4Manager: try: table_entry.insert() LOGGER.info("Inserted exact table entry: %s", table_entry) - except (P4RuntimeException, P4RuntimeWriteException) as ex: - ex_msg = str(ex) - LOGGER.warning(ex) + except Exception as ex: + LOGGER.info("Failed to insert exact table entry in table: %s", table_name) + table_entry.modify() + LOGGER.info("Modified table entry in table: %s", table_name) + except (P4RuntimeException, P4RuntimeWriteException) as pex: + ex_msg = str(pex) + LOGGER.warning(pex) # Table entry exists, needs to be modified if "ALREADY_EXISTS" in ex_msg: @@ -775,7 +803,7 @@ class P4Manager: return table_entry def insert_table_entry_ternary(self, - table_name, match_map, action_name, action_params, metadata, + table_name, match_map, action_name, action_params, member_id, metadata, priority, cnt_pkt=-1, cnt_byte=-1): """ Insert an entry into a ternary match table. @@ -791,9 +819,22 @@ class P4Manager: :return: inserted entry """ assert match_map, "Table entry without match operations is not accepted" - assert action_name, "Table entry without action is not accepted" - table_entry = TableEntry(self.local_client, table_name)(action=action_name) + LOGGER.info(f" Table name: {table_name}") + LOGGER.info(f" Table type: ternary") + LOGGER.info(f"Action Name: {action_name}") + LOGGER.info(f" Match: {match_map}") + LOGGER.info(f" Member ID: {member_id}") + LOGGER.info(f" Action: {action_params}") + + table_entry = None + if action_name: + table_entry = TableEntry(self.local_client, table_name)(action=action_name) + else: + table_entry = TableEntry(self.local_client, table_name) + + if member_id > 0: + table_entry.member_id = member_id for match_k, match_v in match_map.items(): table_entry.match[match_k] = match_v @@ -843,9 +884,6 @@ class P4Manager: :param cnt_byte: byte count :return: inserted entry """ - assert match_map, "Table entry without match operations is not accepted" - assert action_name, "Table entry without action is not accepted" - raise NotImplementedError( "Range-based table insertion not implemented yet") @@ -865,14 +903,11 @@ class P4Manager: :param cnt_byte: byte count :return: inserted entry """ - assert match_map, "Table entry without match operations is not accepted" - assert action_name, "Table entry without action is not accepted" - raise NotImplementedError( "Optional-based table insertion not implemented yet") def insert_table_entry(self, table_name, - match_map, action_name, action_params, + match_map, action_name, action_params, member_id, priority, metadata=None, cnt_pkt=-1, cnt_byte=-1): """ Insert an entry into a P4 table. @@ -909,14 +944,14 @@ class P4Manager: # Exact match is supported if table_type == p4info_pb2.MatchField.EXACT: return self.insert_table_entry_exact( - table_name, match_map, action_name, action_params, metadata, + table_name, match_map, action_name, action_params, member_id, metadata, cnt_pkt, cnt_byte) # Ternary and LPM matches are supported if table_type in \ [p4info_pb2.MatchField.TERNARY, p4info_pb2.MatchField.LPM]: return self.insert_table_entry_ternary( - table_name, match_map, action_name, action_params, metadata, + table_name, match_map, action_name, action_params, member_id, metadata, priority, cnt_pkt, cnt_byte) # TODO: Cover RANGE match # pylint: disable=W0511 @@ -933,8 +968,8 @@ class P4Manager: return None - def delete_table_entry(self, table_name, - match_map, action_name, action_params, priority=0): + def delete_table_entry(self, table_name, match_map, + action_name, action_params, member_id, priority=0): """ Delete an entry from a P4 table. @@ -956,7 +991,11 @@ class P4Manager: LOGGER.error(msg) raise UserError(msg) - table_entry = TableEntry(self.local_client, table_name)(action=action_name) + table_entry = None + if action_name: + table_entry = TableEntry(self.local_client, table_name)(action=action_name) + else: + table_entry = TableEntry(self.local_client, table_name) for match_k, match_v in match_map.items(): table_entry.match[match_k] = match_v @@ -1010,7 +1049,12 @@ class P4Manager: LOGGER.error(msg) raise UserError(msg) - TableEntry(self.local_client, table_name).read(function=lambda x: x.delete()) + try: + TableEntry(self.local_client, table_name).read(function=lambda x: x.delete()) + except P4RuntimeException as ex: + LOGGER.error(f"Failed to delete table entries for table {table_name}") + raise P4RuntimeException from ex + LOGGER.info("Deleted all entries from table: %s", table_name) def print_table_entries_spec(self, table_name): @@ -1243,11 +1287,16 @@ class P4Manager: continue cnt_res["counter-name"] = cnt_name + cnt_res["entries"] = [] for ent in entries: - cnt_res["index"] = ent.index - cnt_res["packet-count"] = ent.packet_count - cnt_res["byte-count"] = ent.byte_count + cnt_res["entries"].append( + { + "index": ent.index, + "packet-count": ent.packet_count, + "byte-count": ent.byte_count + } + ) return cnt_res @@ -1341,7 +1390,12 @@ class P4Manager: if cnt_byte > 0: cnt_entry.byte_count = cnt_byte - cnt_entry.modify() + try: + cnt_entry.modify() + except P4RuntimeException as ex: + LOGGER.warning(f"Failed to modify counter {cnt_name}") + raise P4RuntimeException from ex + LOGGER.info("Updated counter entry: %s", cnt_entry) return cnt_entry @@ -1460,19 +1514,25 @@ class P4Manager: continue d_cnt_res["direct-counter-name"] = d_cnt_name + d_cnt_res["entries"] = [] for ent in entries: - d_cnt_res["match-fields"] = [] + entry_res = { + "match-fields": [], + "priority": ent.priority, + "packet-count": ent.packet_count, + "byte-count": ent.byte_count + } + for k, v in ent.table_entry.match.items(): - d_cnt_res["match-fields"].append( + entry_res["match-fields"].append( { "match-field": k, "match-value": v } ) - d_cnt_res["priority"] = ent.priority - d_cnt_res["packet-count"] = ent.packet_count - d_cnt_res["byte-count"] = ent.byte_count + + d_cnt_res["entries"].append(entry_res) return d_cnt_res @@ -1575,7 +1635,12 @@ class P4Manager: if cnt_byte > 0: d_cnt_entry.byte_count = cnt_byte - d_cnt_entry.modify() + try: + d_cnt_entry.modify() + except P4RuntimeException as ex: + LOGGER.warning(f"Failed to modify direct counter {d_cnt_name}") + raise P4RuntimeException from ex + LOGGER.info("Updated direct counter entry: %s", d_cnt_entry) return d_cnt_entry @@ -1692,13 +1757,18 @@ class P4Manager: continue meter_res["meter-name"] = meter_name + meter_res["entries"] = [] for ent in entries: - meter_res["index"] = ent.index - meter_res["cir"] = ent.cir - meter_res["cburst"] = ent.cburst - meter_res["pir"] = ent.pir - meter_res["pburst"] = ent.pburst + meter_res["entries"].append( + { + "index": ent.index, + "cir": ent.cir, + "cburst": ent.cburst, + "pir": ent.pir, + "pburst": ent.pburst + } + ) return meter_res @@ -1806,7 +1876,12 @@ class P4Manager: if pburst > 0: meter_entry.pburst = pburst - meter_entry.modify() + try: + meter_entry.modify() + except P4RuntimeException as ex: + LOGGER.warning(f"Failed to modify meter {meter_name}") + raise P4RuntimeException from ex + LOGGER.info("Updated meter entry: %s", meter_entry) return meter_entry @@ -1925,20 +2000,26 @@ class P4Manager: continue d_meter_res["direct-meter-name"] = d_meter_name + d_meter_res["entries"] = [] for ent in entries: - d_meter_res["match-fields"] = [] + entry_res = { + "match-fields": [], + "cir": ent.cir, + "cburst": ent.cburst, + "pir": ent.pir, + "pburst": ent.pburst + } + for k, v in ent.table_entry.match.items(): - d_meter_res["match-fields"].append( + entry_res["match-fields"].append( { "match-field": k, "match-value": v } ) - d_meter_res["cir"] = ent.cir - d_meter_res["cburst"] = ent.cburst - d_meter_res["pir"] = ent.pir - d_meter_res["pburst"] = ent.pburst + + d_meter_res["entries"].append(entry_res) return d_meter_res @@ -2049,7 +2130,12 @@ class P4Manager: if pburst > 0: d_meter_entry.pburst = pburst - d_meter_entry.modify() + try: + d_meter_entry.modify() + except P4RuntimeException as ex: + LOGGER.warning(f"Failed to modify direct meter {d_meter_name}") + raise P4RuntimeException from ex + LOGGER.info("Updated direct meter entry: %s", d_meter_entry) return d_meter_entry @@ -2163,23 +2249,32 @@ class P4Manager: if not act_p.name == ap_name: continue + entries = self.get_action_prof_member_entries(ap_name) + if len(entries) == 0: + continue + ap_res["action-profile-name"] = ap_name + ap_res["entries"] = [] - entries = self.get_action_prof_member_entries(ap_name) for ent in entries: action = ent.action action_name = CONTEXT.get_name_from_id(action.id) - ap_res["action"] = action_name - ap_res["action-params"] = [] + + entry_res = { + "action": action_name, + "action-params": [], + "member-id": ent.member_id + } + for k, v in action.items(): - ap_res["action-params"].append( + entry_res["action-params"].append( { "param": k, "value": v } ) - ap_res["member-id"] = ent.member_id + ap_res["entries"].append(entry_res) return ap_res @@ -2241,7 +2336,7 @@ class P4Manager: action_params=action_params ) if operation == WriteOperation.delete: - LOGGER.debug( + LOGGER.info( "Action profile member entry to delete: %s", json_resource) return self.delete_action_prof_member_entry( ap_name=ap_name, @@ -2300,11 +2395,15 @@ class P4Manager: """ act_p = self.get_action_profile(ap_name) assert act_p, \ - "P4 pipeline does not implement action profile " + ap_name + "P4 pipeline does not implement action profile member " + ap_name + + ap_member_entry = ActionProfileMember(self.local_client, ap_name)(member_id=member_id) + try: + ap_member_entry.delete() + except P4RuntimeException as ex: + LOGGER.error(f"Failed to delete action profile member {ap_name} with action {action_name}") + raise P4RuntimeException from ex - ap_member_entry = ActionProfileMember(self.local_client, ap_name)( - member_id=member_id, action=action_name) - ap_member_entry.delete() LOGGER.info("Deleted action profile member entry: %s", ap_member_entry) return ap_member_entry @@ -2454,19 +2553,28 @@ class P4Manager: if not act_p.name == ap_name: continue + entries = self.get_action_prof_group_entries(ap_name) + if len(entries) == 0: + continue + ap_res["action-profile-name"] = ap_name + ap_res["entries"] = [] - entries = self.get_action_prof_group_entries(ap_name) for ent in entries: - ap_res["group-id"] = ent.group_id - ap_res["members"] = [] + entry_res = { + "group-id": ent.group_id, + "members": [] + } + for mem in ent.members: - ap_res["members"].append( + entry_res["members"].append( { "member": mem } ) + ap_res["entries"].append(entry_res) + return ap_res def action_prof_group_entry_operation_from_json(self, @@ -2556,7 +2664,14 @@ class P4Manager: "P4 pipeline does not implement action profile " + ap_name ap_group_entry = ActionProfileGroup(self.local_client, ap_name)(group_id=group_id) - ap_group_entry.delete() + try: + ap_group_entry.delete() + except P4RuntimeException as ex: + LOGGER.error( + f"Failed to delete action profile group {group_id} " + f"of action profile {ap_name}") + raise P4RuntimeException from ex + LOGGER.info("Deleted action profile group entry: %s", ap_group_entry) return ap_group_entry @@ -2699,19 +2814,15 @@ class P4Manager: mcast_res = {} mcast_res["group-id"] = mcast_group.group_id - mcast_res["egress-ports"] = [] - mcast_res["instances"] = [] + mcast_res["replicas"] = [] for r in mcast_group.replicas: - mcast_res["egress-ports"].append( - { - "egress-port": r.egress_port - } - ) - mcast_res["instances"].append( + mcast_res["replicas"].append( { + "egress-port": r.egress_port, "instance": r.instance } ) + mcast_list_res.append(mcast_res) return mcast_list_res @@ -2793,12 +2904,17 @@ class P4Manager: "Multicast group " + group_id + " must be > 0" mcast_group = MulticastGroupEntry(self.local_client, group_id) - mcast_group.delete() + + try: + mcast_group.delete() + except P4RuntimeException as ex: + LOGGER.error(f"Failed to delete multicast group {group_id}") + raise P4RuntimeException from ex if group_id in self.multicast_groups: del self.multicast_groups[group_id] - LOGGER.info( - "Deleted multicast group %d", group_id) + + LOGGER.info("Deleted multicast group %d", group_id) return mcast_group @@ -2810,7 +2926,11 @@ class P4Manager: """ for mcast_group in MulticastGroupEntry(self.local_client).read(): gid = mcast_group.group_id - mcast_group.delete() + try: + mcast_group.delete() + except P4RuntimeException as ex: + LOGGER.error(f"Failed to delete multicast group {gid}") + raise P4RuntimeException from ex del self.multicast_groups[gid] assert self.count_multicast_groups() == 0, \ @@ -2896,19 +3016,15 @@ class P4Manager: session_res = {} session_res["session-id"] = session.session_id - session_res["egress-ports"] = [] - session_res["instances"] = [] + session_res["replicas"] = [] for r in session.replicas: - session_res["egress-ports"].append( - { - "egress-port": r.egress_port - } - ) - session_res["instances"].append( + session_res["replicas"].append( { + "egress-port": r.egress_port, "instance": r.instance } ) + session_list_res.append(session_res) return session_list_res @@ -2996,13 +3112,15 @@ class P4Manager: try: session.delete() - LOGGER.info("Deleted clone session %d", session_id) except (P4RuntimeException, P4RuntimeWriteException) as ex: - LOGGER.error(ex) + LOGGER.error(f"Failed to delete clone session {session_id}") + raise P4RuntimeException from ex if session_id in self.clone_session_entries: del self.clone_session_entries[session_id] + LOGGER.info("Deleted clone session %d", session_id) + return session def delete_clone_session_entries(self): @@ -3013,7 +3131,12 @@ class P4Manager: """ for e in CloneSessionEntry(self.local_client).read(): sid = e.session_id - e.delete() + try: + e.delete() + except P4RuntimeException as ex: + LOGGER.error(f"Failed to delete clone session {sid}") + raise P4RuntimeException from ex + del self.clone_session_entries[sid] assert self.count_multicast_groups() == 0, \ -- GitLab