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

Fixes log level for got supported group message #8207

Merged
merged 3 commits into from
Sep 15, 2023

Conversation

mcagriaksoy
Copy link

@mcagriaksoy mcagriaksoy commented Sep 13, 2023

Description

This is the copy PR of the previous #7314
somehow I cannot re-open that.

This PR resolve #6765 issue. During one successful DTLS handshake, user seeing many message caused by wrong log level.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

  • changelog Provided
  • backport not needed (code not present in 2.28)
  • tests not required

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@gilles-peskine-arm gilles-peskine-arm added bug needs-review Every commit must be reviewed by at least two team members, component-tls priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most) labels Sep 14, 2023
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Looks good to me except that it should have a changelog entry since this is a reported bug.

I've checked that this debug line isn't present in 2.28 (it was added in 3.2), so no backport is needed.

library/ssl_client.c Show resolved Hide resolved
@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Sep 14, 2023
Signed-off-by: mcagriaksoy <mcagriaksoy@yandex.com>
@gilles-peskine-arm
Copy link
Contributor

Please don't include merge commits in pull requests unless necessary to resolve a merge conflict with a complicated history, or a CI failure. They are time-consuming to review.

xkqian
xkqian previously approved these changes Sep 15, 2023
Copy link
Contributor

@xkqian xkqian left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Sep 15, 2023
@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-work labels Sep 15, 2023
@@ -0,0 +1,2 @@
Bugfix
* Fix log level for the got supported group message. Fixes #6765
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment in the PR is much more useful than this, so could you change this to something like the following (also note the final period/full stop).

   * Fix an incorrect log level that was causing excessive messages during
     DTLS handshakes. Fixes #6765.

Signed-off-by: mcagriaksoy <mcagriaksoy@yandex.com>
Merged via the queue into Mbed-TLS:development with commit 170be45 Sep 15, 2023
1 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug component-tls priority-medium Medium priority - this can be reviewed as time permits size-xs Estimated task size: extra small (a few hours at most)
Projects
Development

Successfully merging this pull request may close these issues.

Log level for "got supported group" message is set to Error
4 participants