From 7723ca15ea157e88ec3b548908037971d08040fe Mon Sep 17 00:00:00 2001 From: Fan Kong Date: Fri, 13 Mar 2026 15:12:05 -0700 Subject: [PATCH 1/4] Add PR body template and instructions for upstream sonic-net/sonic-bmp PR (modeled after whitebox PR #2) Made-with: Cursor --- INSTRUCTIONS_OPEN_PR.md | 49 +++++++++++++++++++++++++++++++++++++++++ PR_BODY.md | 33 +++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 INSTRUCTIONS_OPEN_PR.md create mode 100644 PR_BODY.md diff --git a/INSTRUCTIONS_OPEN_PR.md b/INSTRUCTIONS_OPEN_PR.md new file mode 100644 index 0000000..6e58df9 --- /dev/null +++ b/INSTRUCTIONS_OPEN_PR.md @@ -0,0 +1,49 @@ +# How to open a PR to github.com/sonic-net/sonic-bmp (master) + +## 1. Apply your changes (from Cisco whitebox/sonic-bmp PR #2) + +Copy the code changes from: +**https://wwwin-github.cisco.com/whitebox/sonic-bmp/pull/2** + +Into this clone: +```bash +cd /nobackup/fkong/sonic-bmp-upstream +# Apply patch, or copy changed files, then: +git add -A +git status +git commit -m "Your commit message (match or summarize Cisco PR #2)" +``` + +## 2. Fork sonic-net/sonic-bmp (if you have not already) + +- Go to https://github.com/sonic-net/sonic-bmp +- Click **Fork** to create your fork (e.g. `https://github.com/YOUR_USERNAME/sonic-bmp`) + +## 3. Add your fork and push the branch + +```bash +cd /nobackup/fkong/sonic-bmp-upstream +git remote add myfork https://github.com/YOUR_USERNAME/sonic-bmp.git # or git@github.com:YOUR_USERNAME/sonic-bmp.git +git push -u myfork cisco-sync-pr2-model +``` + +Replace `YOUR_USERNAME` with your GitHub username. + +## 4. Open the Pull Request + +1. Go to: **https://github.com/sonic-net/sonic-bmp/compare** +2. Set **base repository** to `sonic-net/sonic-bmp`, **base** to `master` +3. Set **head repository** to your fork, **compare** to `cisco-sync-pr2-model` +4. Click **Create pull request** +5. Paste the contents of `PR_BODY.md` (after filling it from Cisco PR #2) into the PR description + +Direct link (after pushing): +**https://github.com/sonic-net/sonic-bmp/compare/master...YOUR_USERNAME:sonic-bmp:cisco-sync-pr2-model** + +## Branch and base + +| Item | Value | +|--------|--------| +| Your branch | `cisco-sync-pr2-model` | +| Base repo | `sonic-net/sonic-bmp` | +| Base branch | `master` | diff --git a/PR_BODY.md b/PR_BODY.md new file mode 100644 index 0000000..0cab84a --- /dev/null +++ b/PR_BODY.md @@ -0,0 +1,33 @@ +# Pull Request - Sync from whitebox/sonic-bmp (modeled after PR #2) + +## Summary + + +## Type of change +- [ ] Bug fix +- [ ] New feature +- [ ] Code refactor +- [ ] Documentation update +- [ ] Other (describe): + +## Motivation and Context + + +## Approach + + +## How has this been tested? + + +## Checklist +- [ ] My code follows the project's style guidelines +- [ ] I have performed a self-review of my code +- [ ] I have commented my code where necessary +- [ ] I have updated the documentation accordingly +- [ ] My changes generate no new warnings +- [ ] I have added tests that prove my fix/feature works + +--- +**Base repository:** https://github.com/sonic-net/sonic-bmp +**Base branch:** `master` +**Source:** Modeled after wwwin-github.cisco.com/whitebox/sonic-bmp/pull/2 From a0a49bfbf7794b8e6473d4181bf343dcdc09ca80 Mon Sep 17 00:00:00 2001 From: Fan Kong Date: Fri, 13 Mar 2026 15:19:28 -0700 Subject: [PATCH 2/4] Move BMPReader rBMP above try block for correct scope Keep rBMP in scope for the full ClientThread lifetime so the std::thread that uses &rBMP does not reference an object that has gone out of scope when the try block exits. Moved from inside try to just after pthread_cleanup_push. Made-with: Cursor --- Server/src/client_thread.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Server/src/client_thread.cpp b/Server/src/client_thread.cpp index 4b72eda..d1f4067 100644 --- a/Server/src/client_thread.cpp +++ b/Server/src/client_thread.cpp @@ -96,6 +96,11 @@ void *ClientThread(void *arg) { */ pthread_cleanup_push(ClientThread_cancel, &cInfo); + /* + * moved rBMP here to keep it in context for the try ... catch + */ + BMPReader rBMP(logger, thr->cfg); + try { #ifndef REDIS_ENABLED // connect to message bus @@ -108,7 +113,7 @@ void *ClientThread(void *arg) { cInfo.redis = std::make_shared(logger, thr->cfg, cInfo.client); cInfo.redis->ResetAllTables(); #endif - BMPReader rBMP(logger, thr->cfg); + //BMPReader rBMP(logger, thr->cfg); --- moved up to keep rBMP in context LOG_INFO("Thread started to monitor BMP from router %s using socket %d buffer in bytes = %u", cInfo.client->c_ip, cInfo.client->c_sock, thr->cfg->bmp_buffer_size); @@ -122,7 +127,7 @@ void *ClientThread(void *arg) { */ bool bmp_run = true; #ifndef REDIS_ENABLED - //cInfo.bmp_reader_thread = new std::thread([&] {rBMP.readerThreadLoop(bmp_run,cInfo.client, + //cInfo.bmp_reader_thread = new std::thread([&] rBMP.readerThreadLoop(bmp_run,cInfo.client, cInfo.bmp_reader_thread = new std::thread(&BMPReader::readerThreadLoop, &rBMP, std::ref(bmp_run), cInfo.client, (MsgBusInterface *)cInfo.mbus ); #else From 665c719d2a4c222a702399dca1f86e251560da5b Mon Sep 17 00:00:00 2001 From: Fan Kong Date: Fri, 13 Mar 2026 15:20:00 -0700 Subject: [PATCH 3/4] Update PR body and instructions for upstream PR Made-with: Cursor --- INSTRUCTIONS_OPEN_PR.md | 22 +++++----------------- PR_BODY.md | 16 ++++++++-------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/INSTRUCTIONS_OPEN_PR.md b/INSTRUCTIONS_OPEN_PR.md index 6e58df9..fe6ab96 100644 --- a/INSTRUCTIONS_OPEN_PR.md +++ b/INSTRUCTIONS_OPEN_PR.md @@ -1,25 +1,13 @@ # How to open a PR to github.com/sonic-net/sonic-bmp (master) -## 1. Apply your changes (from Cisco whitebox/sonic-bmp PR #2) +Patch is committed on branch `cisco-sync-pr2-model`. Follow steps below to push and open the PR. -Copy the code changes from: -**https://wwwin-github.cisco.com/whitebox/sonic-bmp/pull/2** - -Into this clone: -```bash -cd /nobackup/fkong/sonic-bmp-upstream -# Apply patch, or copy changed files, then: -git add -A -git status -git commit -m "Your commit message (match or summarize Cisco PR #2)" -``` - -## 2. Fork sonic-net/sonic-bmp (if you have not already) +## 1. Fork sonic-net/sonic-bmp (if you have not already) - Go to https://github.com/sonic-net/sonic-bmp - Click **Fork** to create your fork (e.g. `https://github.com/YOUR_USERNAME/sonic-bmp`) -## 3. Add your fork and push the branch +## 2. Add your fork and push the branch ```bash cd /nobackup/fkong/sonic-bmp-upstream @@ -29,13 +17,13 @@ git push -u myfork cisco-sync-pr2-model Replace `YOUR_USERNAME` with your GitHub username. -## 4. Open the Pull Request +## 3. Open the Pull Request 1. Go to: **https://github.com/sonic-net/sonic-bmp/compare** 2. Set **base repository** to `sonic-net/sonic-bmp`, **base** to `master` 3. Set **head repository** to your fork, **compare** to `cisco-sync-pr2-model` 4. Click **Create pull request** -5. Paste the contents of `PR_BODY.md` (after filling it from Cisco PR #2) into the PR description +5. Paste the contents of `PR_BODY.md` into the PR description (already filled) Direct link (after pushing): **https://github.com/sonic-net/sonic-bmp/compare/master...YOUR_USERNAME:sonic-bmp:cisco-sync-pr2-model** diff --git a/PR_BODY.md b/PR_BODY.md index 0cab84a..2cab7a2 100644 --- a/PR_BODY.md +++ b/PR_BODY.md @@ -1,23 +1,24 @@ -# Pull Request - Sync from whitebox/sonic-bmp (modeled after PR #2) +# Move BMPReader rBMP above try block for correct scope ## Summary - +In `ClientThread()`, `BMPReader rBMP` is used by a `std::thread` that receives `&rBMP`. Previously `rBMP` was declared inside the `try` block, so it could go out of scope before the thread finished. This change moves the declaration above the `try` so `rBMP` stays in scope for the full thread lifetime. ## Type of change -- [ ] Bug fix +- [x] Bug fix - [ ] New feature - [ ] Code refactor - [ ] Documentation update - [ ] Other (describe): ## Motivation and Context - +The reader thread runs `BMPReader::readerThreadLoop` with a pointer to `rBMP`. If `rBMP` is only in scope inside the `try` block, it is destroyed when the block exits (e.g. on exception or normal path), while the thread may still be running, leading to use-after-free or undefined behavior. Moving `rBMP` above the `try` keeps it in context for the entire `ClientThread` function and the cleanup handler. ## Approach - +- Declare `BMPReader rBMP(logger, thr->cfg);` immediately after `pthread_cleanup_push(ClientThread_cancel, &cInfo);` and before `try`. +- Remove the declaration from inside the `try` block (left as a comment for context). ## How has this been tested? - +- Code review; build can be verified via `make` in a environment with cmake and dependencies (librdkafka, libyaml-cpp). Upstream Azure Pipelines will build on PR. ## Checklist - [ ] My code follows the project's style guidelines @@ -29,5 +30,4 @@ --- **Base repository:** https://github.com/sonic-net/sonic-bmp -**Base branch:** `master` -**Source:** Modeled after wwwin-github.cisco.com/whitebox/sonic-bmp/pull/2 +**Base branch:** `master` From c71ffebdc7e75fca6396c0d6ddefb1d0ba0f39f8 Mon Sep 17 00:00:00 2001 From: Fan Kong Date: Fri, 13 Mar 2026 15:36:33 -0700 Subject: [PATCH 4/4] remove md --- INSTRUCTIONS_OPEN_PR.md | 37 ------------------------------------- PR_BODY.md | 33 --------------------------------- 2 files changed, 70 deletions(-) delete mode 100644 INSTRUCTIONS_OPEN_PR.md delete mode 100644 PR_BODY.md diff --git a/INSTRUCTIONS_OPEN_PR.md b/INSTRUCTIONS_OPEN_PR.md deleted file mode 100644 index fe6ab96..0000000 --- a/INSTRUCTIONS_OPEN_PR.md +++ /dev/null @@ -1,37 +0,0 @@ -# How to open a PR to github.com/sonic-net/sonic-bmp (master) - -Patch is committed on branch `cisco-sync-pr2-model`. Follow steps below to push and open the PR. - -## 1. Fork sonic-net/sonic-bmp (if you have not already) - -- Go to https://github.com/sonic-net/sonic-bmp -- Click **Fork** to create your fork (e.g. `https://github.com/YOUR_USERNAME/sonic-bmp`) - -## 2. Add your fork and push the branch - -```bash -cd /nobackup/fkong/sonic-bmp-upstream -git remote add myfork https://github.com/YOUR_USERNAME/sonic-bmp.git # or git@github.com:YOUR_USERNAME/sonic-bmp.git -git push -u myfork cisco-sync-pr2-model -``` - -Replace `YOUR_USERNAME` with your GitHub username. - -## 3. Open the Pull Request - -1. Go to: **https://github.com/sonic-net/sonic-bmp/compare** -2. Set **base repository** to `sonic-net/sonic-bmp`, **base** to `master` -3. Set **head repository** to your fork, **compare** to `cisco-sync-pr2-model` -4. Click **Create pull request** -5. Paste the contents of `PR_BODY.md` into the PR description (already filled) - -Direct link (after pushing): -**https://github.com/sonic-net/sonic-bmp/compare/master...YOUR_USERNAME:sonic-bmp:cisco-sync-pr2-model** - -## Branch and base - -| Item | Value | -|--------|--------| -| Your branch | `cisco-sync-pr2-model` | -| Base repo | `sonic-net/sonic-bmp` | -| Base branch | `master` | diff --git a/PR_BODY.md b/PR_BODY.md deleted file mode 100644 index 2cab7a2..0000000 --- a/PR_BODY.md +++ /dev/null @@ -1,33 +0,0 @@ -# Move BMPReader rBMP above try block for correct scope - -## Summary -In `ClientThread()`, `BMPReader rBMP` is used by a `std::thread` that receives `&rBMP`. Previously `rBMP` was declared inside the `try` block, so it could go out of scope before the thread finished. This change moves the declaration above the `try` so `rBMP` stays in scope for the full thread lifetime. - -## Type of change -- [x] Bug fix -- [ ] New feature -- [ ] Code refactor -- [ ] Documentation update -- [ ] Other (describe): - -## Motivation and Context -The reader thread runs `BMPReader::readerThreadLoop` with a pointer to `rBMP`. If `rBMP` is only in scope inside the `try` block, it is destroyed when the block exits (e.g. on exception or normal path), while the thread may still be running, leading to use-after-free or undefined behavior. Moving `rBMP` above the `try` keeps it in context for the entire `ClientThread` function and the cleanup handler. - -## Approach -- Declare `BMPReader rBMP(logger, thr->cfg);` immediately after `pthread_cleanup_push(ClientThread_cancel, &cInfo);` and before `try`. -- Remove the declaration from inside the `try` block (left as a comment for context). - -## How has this been tested? -- Code review; build can be verified via `make` in a environment with cmake and dependencies (librdkafka, libyaml-cpp). Upstream Azure Pipelines will build on PR. - -## Checklist -- [ ] My code follows the project's style guidelines -- [ ] I have performed a self-review of my code -- [ ] I have commented my code where necessary -- [ ] I have updated the documentation accordingly -- [ ] My changes generate no new warnings -- [ ] I have added tests that prove my fix/feature works - ---- -**Base repository:** https://github.com/sonic-net/sonic-bmp -**Base branch:** `master`