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.

Sign up for GitHub

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

Program nexthop table as a ternary match. #59

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Program nexthop table as a ternary match. #59

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
910287d
Program nexthop table as a ternary match.
n-sandeep Oct 9, 2023
4751c74
Condition based TDI call for nexthop table match type.
n-sandeep Oct 11, 2023
962c0b5
Update README.md
ffoulkes Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,15 @@
# ipdk-io/krnlmon
The Kernel Monitor receives RFC 3549 messages from the Linux Kernel over a Netlink socket when changes are made to the kernel networking data structures.

## Breaking changes

Copy link
Contributor

@ffoulkes ffoulkes Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded for use in this context.

Do we need to say which P4 program?

Since there is a change in p4 program, where nexthop table has been moved to
WCM block from SEM block (exact match). TDI provides seperate API's for each
block. Since Ternary/WCM needs a MASK to be populated we need to use a newer
API which can take mask as a parameter. Changes with a MACRO has been
introduced in `switchapi/es2k/switch_pd_routing.c`, which defines ternary match
MACRO enablement. If user wants to pick older p4 program, then user need to
modify this MACRO value to 0 and re-build infrap4d to be compatible with
older Exact match type for nexthop table.

PR: https://github.com/ipdk-io/krnlmon/pull/59 introduces this breakage changes.
13 changes: 13 additions & 0 deletions switchapi/es2k/switch_pd_routing.c
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the short term:

  1. Add #define NEXTHOP_TABLE_TERNARY_MATCH 1 at the top of this file.
  2. Instead of replacing the code outright, change it to something like this:
#if NEXTHOP_TABLE_TERNARY_MATCH
    // new p4 program
    status = tdi_key_field_set_value_and_mask(key_hdl, field_id,
                                     (api_nexthop_pd_info->nexthop_handle &
                                     ~(SWITCH_HANDLE_TYPE_NHOP <<
                                     SWITCH_HANDLE_TYPE_SHIFT)), 0xFFFF);
#else
    // legacy p4 program
    status = tdi_key_field_set_value(key_hdl, field_id,
                                     (api_nexthop_pd_info->nexthop_handle &
                                     ~(SWITCH_HANDLE_TYPE_NHOP <<
                                     SWITCH_HANDLE_TYPE_SHIFT)));
#endif
  1. Provide a Git commit comment saying flat-out that this commit is a breaking change. Recap the problem (a change in the P4 program [because...]), say what you changed, and state briefly what the side effects of the change are.
  2. Advertise the fact that there's a breaking change. For starters, I'd add a *Breaking Changes to the top-level krnlmon README.md file. Say what was done and what the user should do about it. Make the change part of this commit.
  3. We'll want to add a Breaking Changes notice to networking-recipe as well, when we update the submodule reference.

Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@

#include "port_mgr/dpdk/bf_dpdk_port_if.h"

// Table match type definitions.
#define NEXTHOP_TABLE_TERNARY_MATCH 1

switch_status_t switch_routing_table_entry (
switch_device_t device,
const switch_pd_routing_info_t *api_routing_info,
Expand Down Expand Up @@ -180,10 +183,20 @@ switch_status_t switch_pd_nexthop_table_entry(
goto dealloc_resources;
}

#if NEXTHOP_TABLE_TERNARY_MATCH
// When Nexthop table is of type ternary Match
status = tdi_key_field_set_value_and_mask(key_hdl, field_id,
(api_nexthop_pd_info->nexthop_handle &
~(SWITCH_HANDLE_TYPE_NHOP <<
SWITCH_HANDLE_TYPE_SHIFT)), 0xFFFF);
#else
// When Nexthop table is of type exact Match
status = tdi_key_field_set_value(key_hdl, field_id,
(api_nexthop_pd_info->nexthop_handle &
~(SWITCH_HANDLE_TYPE_NHOP <<
SWITCH_HANDLE_TYPE_SHIFT)));
#endif

if (status != TDI_SUCCESS) {
krnlmon_log_error("Unable to set value for key ID: %d for nexthop_table,"
" error: %d", field_id, status);
Expand Down