Fix the bcmsh issues. #1761
Merged
Fix the bcmsh issues. #1761
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qiluo-msft
reviewed
Jun 5, 2018
| @@ -20,7 +20,7 @@ debs/{{ deb }}{{' '}} | |||
| RUN apt-get install -f kmod | |||
|
|
|||
| COPY ["files/dsserve", "files/bcmcmd", "start.sh", "bcmsh", "/usr/bin/"] | |||
| RUN chmod +x /usr/bin/dsserve /usr/bin/bcmcmd | |||
| RUN chmod +x /usr/bin/dsserve /usr/bin/bcmcmd /usr/bin/bcmsh | |||
There was a problem hiding this comment.
x [](start = 11, length = 1)
Are you sure? The original bcmsh used in build is executable.
qiluo-msft
reviewed
Jun 5, 2018
| @@ -17,3 +17,4 @@ $(DOCKER_SYNCD_BRCM)_RUN_OPT += -v /var/run/docker-syncd:/var/run/sswsyncd | |||
| $(DOCKER_SYNCD_BRCM)_RUN_OPT += -v /etc/sonic:/etc/sonic:ro | |||
|
|
|||
| $(DOCKER_SYNCD_BRCM)_BASE_IMAGE_FILES += bcmcmd:/usr/bin/bcmcmd | |||
| $(DOCKER_SYNCD_BRCM)_BASE_IMAGE_FILES += bcmsh:/usr/bin/bcmsh | |||
There was a problem hiding this comment.
bcmsh [](start = 56, length = 5)
I think bcmsh script will not work in host, because the domain socket is in a different place there. #Closed
There was a problem hiding this comment.
Disagree here. The script in the change is not the original script inside docker, it is as below:
#!/bin/bash
docker exec -i syncd bcmsh "$@"
It calls the script inside the docker and the same domain socket is used.
Everything works fine from host side, and people don't need go to docker to type the command.
#Closed
There was a problem hiding this comment.
There was a problem hiding this comment.
still marked as "Changes requested", can you remove that? #Closed
-- bcmsh is not copied to /usr/bin/ at host side
zhenggen-xu
force-pushed
the
github-fork-bcmsh
branch
from
7f74ad1
to
b58a94d
Compare
qiluo-msft
approved these changes
Jun 5, 2018
qiluo-msft
pushed a commit
that referenced
this pull request
Sep 1, 2021
#### Why I did it Fixing issue [[sonic-utilities] Unit test failed when building sonic-utilities #1761](sonic-net/sonic-utilities#1761) Importing `sonic-acl` caused getting references by `backlinks()` to break, #### How I did it solution is to comment out the importing statement as it is not used anyway. #### How to verify it Ran sonic-utilities unit-tests locally after the fix, and all passed.
volodymyrsamotiy
added a commit
to volodymyrsamotiy/sonic-buildimage
that referenced
this pull request
Sep 3, 2021
* d240291 Update port_rates & rif_rates lua scripts to convert poll_interval to MS (sonic-net#1855) * a71a5d3 [acl mirror action] Mirror session ref count fix at acl rule attachment (sonic-net#1761) * 197f427 Fix vs test failure in test_buffer_traditional (sonic-net#1881) * 8471f42 Revert "[debugcounterorch] check if counter type is supported before querying… (sonic-net#1789)" (sonic-net#1884) Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
5 tasks
prsunny
pushed a commit
that referenced
this pull request
Sep 3, 2021
* d240291 Update port_rates & rif_rates lua scripts to convert poll_interval to MS (#1855) * a71a5d3 [acl mirror action] Mirror session ref count fix at acl rule attachment (#1761) * 197f427 Fix vs test failure in test_buffer_traditional (#1881) * 8471f42 Revert "[debugcounterorch] check if counter type is supported before querying… (#1789)" (#1884) Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
theasianpianist
pushed a commit
to theasianpianist/sonic-buildimage
that referenced
this pull request
Feb 5, 2022
…nt (sonic-net#1761) * Fix mirror session ref count at acl rule attachement Signed-off-by: wenda.ni <wenda.ni@bytedance.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
-- bcmsh in docker is not executable.
-- bcmsh is not copied to /usr/bin/ at host side
- What I did
Fix the issues with bcmsh:
--bcmsh is not executable today in docker, so if you type bcmsh, it won't work.
--bcmsh is not at /usr/bin so you can not type that command directly.
- How I did it
Fix above two issues.
- How to verify it
bcmsh is working at host and docker.
admin@lnos-x1-a-asw03:~$ bcmsh
Press Enter to show prompt.
Press Ctrl+C to exit.
drivshell>
- Description for the changelog
Fix bcmsh issues (#1761)
- A picture of a cute animal (not mandatory but encouraged)