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

[202305] update frr patch for bgpd-Ensure-community-data-is-freed-in-some-cases #18246

Merged

Conversation

lipxu
Copy link
Contributor

@lipxu lipxu commented Mar 4, 2024

Why I did it

FRR issue
FRRouting/frr#15459
Add patch for FRR PR for 202305
FRRouting/frr#15466

Work item tracking
  • Microsoft ADO (number only):
  • 26876083

How I did it

Add patch for FRR PR
FRRouting/frr#15466

How to verify it

Run the original case locally

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@lipxu lipxu requested a review from lguohan as a code owner March 4, 2024 07:10
@yxieca
Copy link
Contributor

yxieca commented Mar 4, 2024

@StormLiangMS the change looks good. but I thought patch 38 can be merged with patch 37 as it has been taken?

@dgsudharsan
Copy link
Collaborator

@StormLiangMS the change looks good. but I thought patch 38 can be merged with patch 37 as it has been taken?

Agree with Ying. I would prefer to have a single patch. Can we update?

@StormLiangMS StormLiangMS reopened this Mar 5, 2024
- bgp_adj_out_set_subgroup(
- dest, subgrp, &attr, selected);
- else
+ if (!bgp_check_withdrawal(bgp, dest)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please check the latest update on FRRouting/frr#15466 ? The diff has changed in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please check the latest update on FRRouting/frr#15466 ? The diff has changed in this file

HI, @dgsudharsan thanks for reviewing the code. This part of the code should be the same, the difference should be related to the difference between 202305 branch and master. thanks.

if (dest) {
for (pi = bgp_dest_get_bgp_path_info(dest); pi;
pi = pi->next) {
- if (CHECK_FLAG(pi->flags, BGP_PATH_SELECTED))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please check the original PR for this file as well? The diff is little bit different

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please check the original PR for this file as well? The diff is little bit different

Yes, this diff was based on our understanding. we will sync with donaldsharp first, and push the latest update if necessary later, thanks.

@StormLiangMS
Copy link
Contributor

PR test pending due to maintainess, bypass the not finished test.

@StormLiangMS StormLiangMS merged commit 524dea6 into sonic-net:202305 Mar 8, 2024
14 of 18 checks passed
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

Successfully merging this pull request may close these issues.

4 participants