[PINS] Add genl_packet kernel module for PINS packet ins #9086

bocon13 · 2021-10-27T07:13:55Z

Add genl_packet kernel module
Extend bcm-knet-cb to support genl_packet packets
Include kernel module in barefoot and broadcom images
Update copp_cfg file with packet in queue configuration

Submission containing materials of a third party:
Copyright Google LLC; Licensed under Apache 2.0 and GPL v2

Co-authored-by: Vivek Ramamoorthy vivekmoorthy@google.com
Co-authored-by: Srikishen Pondicherry Shanmugam kishanps@google.com
Co-authored-by: Yilan Ji yilanji@google.com
Co-authored-by: Brian O'Connor bocon@opennetworking.org

Why I did it

Adds support for receiving packet ins with additonal metadata for PINS.

See this HLD for more details:
https://github.com/pins/SONiC/blob/master/doc/pins/Packet_io.md

How I did it

This code largely follows the same pattern as psample for sflow.

How to verify it

Build a broadcom or barefoot image with INCLUDE_P4RT=y

Install P4RT flow rules for LLDP.

Send LLDP packet from peer using bcmsh:

tx 1 PBM=cd0 DATA="01 80 c2 00 00 0e...

Verify counters and debug logs(dmesg) for knet-cb:

cat /proc/bcm/knet-cb/genl_packet/stats
BCM KNET generic Callback Stats
  DCB type 38
  pkts filter generic cb                  1
  pkts sent to generic module             1
  pkts handled by generic                 1
  pkts with mc destination                0
  pkts with cpu destination               1
  pkts current queue length               0
  pkts high queue length                  1
  pkts drop max queue length              0
  pkts drop no memory                     0
  pkts drop generic not ready             0
  pkts drop metadata parse error          0
  pkts with invalid src port              0
  pkts with invalid dst port              0
  pkts with invalid orig pkt sz           0

Which release branch to backport (provide reason below if selected)

None

Description for the changelog

Add genl_packet kernel module for PINS packet ins

bocon13 · 2021-10-27T07:14:54Z

platform/barefoot/bfn-modules/configs/bfn-modules.conf

+psample
+genl_packet


Ideally this would be conditional on INCLUDE_P4RT

@ravi861, what do you think?

@akokhan @msosyak Please take a look

here is an example how we can do insmod from service installed with a deb. We can add same service into GENL_PACKET deb instead of modifying bfn-modules.conf.
https://github.com/pins/sonic-buildimage-public/blob/master/platform/barefoot/sonic-platform-modules-bfn-newport/configs/systemd/system/bfn-newport.service

@vboykox Can we move both psample and genl_packet there in the future? It should be fine for now just to get things going.

bocon13 · 2021-10-27T07:39:21Z

cc: @vivekmoorthy @kishanps @prsunny

saiarcot895 · 2021-10-27T19:35:37Z

src/genl-packet/debian/rules

+	   echo "Rewriting linux-image dependency in debian/control to $(KVERSION_SHORT)"; \
+	   sed -Ei "s/linux-image-.+-$(ARCH)/linux-image-$(KVERSION_SHORT)-$(ARCH)/" debian/control; \
+	fi
+	if echo "$(KVERSION_SHORT)" | grep -q "^4\.19\."; then \


Why are there two separate if-blocks for this? Wouldn't the previous one handle all cases?

This was a trick that Arista uses:
https://github.com/aristanetworks/sonic/blob/89018d0623191ecd477c869339ca795f91f6c68c/debian/rules#L20-L30

The first if block updates the version string to the kernel version that we are using. The second block covers unsigned kernels if the linux version is 4.19. If the kernel is unsigned, the sed regex in the first block won't match.

You are right that there is some duplication in the second block and this can probably be simplified. Is there simpler common SONiC pattern that we can apply?

The goal here is to avoid updating the debian/control file every time the kernel version is revised.

I haven't tested this, but I think just the first block would be fine. The sed in the first block would still match the linux-image-4.19.0-12-2-amd64 part of linux-image-4.19.0-12-2-amd64-unsigned. I suspect that the second block was added because the name of the package changed to add in -unsigned starting with the 4.19 kernel image.

kishanps · 2021-10-27T23:42:43Z

src/genl-packet/include/uapi/linux/genl-packet.h

+
+enum genl_packet_command {
+  GENL_PACKET_CMD_PACKET,
+};


@bocon13 Per our last conversation, this file was going to go into the user space genetlink lib (p4rt_app/sonic/) ?

bocon13 · 2021-10-29T19:22:24Z

@prsunny To look at the genl_packet common module
@ravi861 To approve bfn changes
@kishanps To find someone from Broadcom to approve bcm-knet-cb changes

files/image_config/copp/copp_cfg.j2

ravi861

OK to commit from BFN.

- Add genl_packet kernel module - Extend bcm-knet-cb to support genl_packet packets - Include kernel module in barefoot and broadcom images - Update copp_cfg file with packet in queue configuration Submission containing materials of a third party: Copyright Google LLC; Licensed under Apache 2.0 and GPL v2 Co-authored-by: Vivek Ramamoorthy <vivekmoorthy@google.com> Co-authored-by: Srikishen Pondicherry Shanmugam <kishanps@google.com> Co-authored-by: Yilan Ji <yilanji@google.com> Co-authored-by: Brian O'Connor <bocon@opennetworking.org> Signed-off-by: Brian O'Connor <bocon@opennetworking.org>

prsunny · 2021-11-01T17:45:41Z

files/image_config/copp/copp_cfg.j2

-
-	    }
+	    }{%- if include_p4rt == "y" %},
+	    "trap.group.cpu.queue.1": {


can we please rename this to the existing format? cpu_queue1_group1, cpu_queue2_group1. We don't need trap.group prefix in the names here

hsincsu · 2022-10-20T03:00:45Z

@bocon13 Hi, I have tried build PINS WITH SONIC, and I want to test pakcetIO with PINS. But I found the genl_pakcet kernel modules still not merged in SONIC. Can I use codes here to build genl_packet.ko for SONIC-202205 version?Thanks a lot for respond.

bocon13 requested review from lguohan, qiluo-msft and xumia as code owners October 27, 2021 07:13

bocon13 commented Oct 27, 2021

View reviewed changes

bocon13 mentioned this pull request Oct 27, 2021

PINS Upstream Tracking for MVP sonic-net/SONiC#841

Open

yxieca requested a review from saiarcot895 October 27, 2021 19:18

saiarcot895 reviewed Oct 27, 2021

View reviewed changes

kishanps reviewed Oct 27, 2021

View reviewed changes

bocon13 commented Oct 29, 2021

View reviewed changes

files/image_config/copp/copp_cfg.j2 Outdated Show resolved Hide resolved

ravi861 previously approved these changes Oct 29, 2021

View reviewed changes

donNewtonAlpha dismissed ravi861’s stale review via d3e4027 October 30, 2021 03:17

donNewtonAlpha force-pushed the pins/genl-packet-kmod branch 2 times, most recently from d3e4027 to c37aeaf Compare October 30, 2021 03:22

donNewtonAlpha force-pushed the pins/genl-packet-kmod branch from c37aeaf to d094581 Compare October 30, 2021 03:28

prsunny reviewed Nov 1, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PINS] Add genl_packet kernel module for PINS packet ins #9086

[PINS] Add genl_packet kernel module for PINS packet ins #9086

bocon13 commented Oct 27, 2021 •

edited

Loading

bocon13 Oct 27, 2021

bocon13 Oct 27, 2021

ravi861 Oct 27, 2021

vboykox Oct 27, 2021

ravi861 Oct 27, 2021

vboykox Oct 27, 2021

bocon13 commented Oct 27, 2021 •

edited

Loading

saiarcot895 Oct 27, 2021

bocon13 Oct 27, 2021 •

edited

Loading

saiarcot895 Oct 27, 2021

kishanps Oct 27, 2021

bocon13 commented Oct 29, 2021

ravi861 left a comment

prsunny Nov 1, 2021

hsincsu commented Oct 20, 2022

[PINS] Add genl_packet kernel module for PINS packet ins #9086

Are you sure you want to change the base?

[PINS] Add genl_packet kernel module for PINS packet ins #9086

Conversation

bocon13 commented Oct 27, 2021 • edited Loading

Why I did it

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Description for the changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bocon13 commented Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

bocon13 Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bocon13 commented Oct 29, 2021

ravi861 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hsincsu commented Oct 20, 2022

bocon13 commented Oct 27, 2021 •

edited

Loading

bocon13 commented Oct 27, 2021 •

edited

Loading

bocon13 Oct 27, 2021 •

edited

Loading