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.

Sign up for GitHub

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

Jump to bottom

Data race between parseBpfData() and LinkDeserialize() #633

Closed
qmonnet opened this issue Apr 6, 2021 · 0 comments · Fixed by #637
Closed

Data race between parseBpfData() and LinkDeserialize() #633

qmonnet opened this issue Apr 6, 2021 · 0 comments · Fixed by #637

Comments

@qmonnet
Copy link

qmonnet commented Apr 6, 2021

Using github.com/vishvananda/netlink v1.1.1-0.20201231054507-6ffafa9fc19b

Go race detector reported a race between netlink.parseBpfData() and netlink.LinkDeserialize().

WARNING: DATA RACE
Write at 0x0000050ade90 by goroutine 237:
  github.com/vishvananda/netlink.parseBpfData()
      /go/src/github.com/cilium/cilium/vendor/github.com/vishvananda/netlink/filter_linux.go:704 +0x9d
  github.com/vishvananda/netlink.(*Handle).FilterList()
      /go/src/github.com/cilium/cilium/vendor/github.com/vishvananda/netlink/filter_linux.go:374 +0x9d5
  github.com/vishvananda/netlink.FilterList()
      /go/src/github.com/cilium/cilium/vendor/github.com/vishvananda/netlink/filter_linux.go:297 +0xf6
  github.com/cilium/cilium/pkg/datapath/loader.RemoveTCFilters()
      /go/src/github.com/cilium/cilium/pkg/datapath/loader/netlink.go:153 +0xb3
  github.com/cilium/cilium/pkg/datapath/loader.(*Loader).reloadDatapath()
      /go/src/github.com/cilium/cilium/pkg/datapath/loader/loader.go:346 +0x166d
  github.com/cilium/cilium/pkg/datapath/loader.(*Loader).ReloadDatapath()
      /go/src/github.com/cilium/cilium/pkg/datapath/loader/loader.go:478 +0x1f5
  github.com/cilium/cilium/pkg/datapath/loader.(*Loader).CompileOrLoad()
      /go/src/github.com/cilium/cilium/pkg/datapath/loader/loader.go:466 +0x704
  github.com/cilium/cilium/pkg/endpoint.(*Endpoint).realizeBPFState()
      /go/src/github.com/cilium/cilium/pkg/endpoint/bpf.go:720 +0x842
  github.com/cilium/cilium/pkg/endpoint.(*Endpoint).regenerateBPF()
      /go/src/github.com/cilium/cilium/pkg/endpoint/bpf.go:635 +0x3f5
  github.com/cilium/cilium/pkg/endpoint.(*Endpoint).regenerate()
      /go/src/github.com/cilium/cilium/pkg/endpoint/policy.go:416 +0xd7b
  github.com/cilium/cilium/pkg/endpoint.(*EndpointRegenerationEvent).Handle()
      /go/src/github.com/cilium/cilium/pkg/endpoint/events.go:65 +0x3e5
  github.com/cilium/cilium/pkg/eventqueue.(*EventQueue).run.func1()
      /go/src/github.com/cilium/cilium/pkg/eventqueue/eventqueue.go:256 +0x26a
  sync.(*Once).doSlow()
      /usr/local/go/src/sync/once.go:68 +0x109
  sync.(*Once).Do()
      /usr/local/go/src/sync/once.go:59 +0x68
  github.com/cilium/cilium/pkg/eventqueue.(*EventQueue).run()
      /go/src/github.com/cilium/cilium/pkg/eventqueue/eventqueue.go:244 +0x64

Previous read at 0x0000050ade90 by goroutine 25:
  github.com/vishvananda/netlink.LinkDeserialize()
      /go/src/github.com/cilium/cilium/vendor/github.com/vishvananda/netlink/link_linux.go:1792 +0x108d
  github.com/vishvananda/netlink.(*Handle).LinkList()
      /go/src/github.com/cilium/cilium/vendor/github.com/vishvananda/netlink/link_linux.go:1937 +0x42a
  github.com/vishvananda/netlink.LinkList()
      /go/src/github.com/cilium/cilium/vendor/github.com/vishvananda/netlink/link_linux.go:1915 +0x65
  github.com/cilium/cilium/pkg/datapath/link.(*ifNameCache).syncCache()
      /go/src/github.com/cilium/cilium/pkg/datapath/link/link.go:90 +0x45
  github.com/cilium/cilium/pkg/datapath/link.init.0.func1()
      /go/src/github.com/cilium/cilium/pkg/datapath/link/link.go:35 +0xc4

Apparently this is about the native variable: filter_linux.go:704:

func parseBpfData(filter Filter, data []syscall.NetlinkRouteAttr) (bool, error) {
	native = nl.NativeEndian()
[...]

and link_linux.go:1792:

func LinkDeserialize(hdr *unix.NlMsghdr, m []byte) (Link, error) {
[...]
		case unix.IFLA_TXQLEN:
			base.TxQLen = int(native.Uint32(attr.Value[0:4]))
[...]
@qmonnet qmonnet mentioned this issue Apr 6, 2021
Closed
tklauser added a commit to tklauser/netlink that referenced this issue Apr 12, 2021
@tklauser
Don't re-initialize package level var native to fix data race
afffe7e 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in (*Handle).filterModify, parseU32Data, parseFwData, parseBpfData
and parseMatchAllData. This fixes a data race between these functions
and any read access of var native, e.g. in LinkDeserialize as reported
in issue vishvananda#633.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
@tklauser tklauser mentioned this issue Apr 12, 2021
Merged
tklauser added a commit to tklauser/netlink that referenced this issue Apr 12, 2021
@tklauser
Don't re-initialize package level var native to fix data race
550f48a 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in (*Handle).filterModify, parseU32Data, parseFwData, parseBpfData
and parseMatchAllData. This fixes a data race between these functions
and any read access of var native, e.g. in LinkDeserialize as reported
in issue vishvananda#633.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to tklauser/netlink that referenced this issue Apr 12, 2021
@tklauser
Don't re-initialize package level var native to fix data race
753a1a4 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue vishvananda#633.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to tklauser/netlink that referenced this issue May 8, 2021
@tklauser
Don't re-initialize package level var native to fix data race
4794848 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue vishvananda#633.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to tklauser/netlink that referenced this issue May 10, 2021
@tklauser
Don't re-initialize or shadow package level var native to fix data race
438a16d 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue vishvananda#633.

Also don't re-declare local variables shadowing the global package-level
var.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
tklauser added a commit to tklauser/netlink that referenced this issue May 10, 2021
@tklauser
Don't re-initialize or shadow package level var native to fix data race
8bc9d0c 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue vishvananda#633.

Also don't re-declare local variables shadowing the global package-level
var.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
aboch pushed a commit that referenced this issue May 10, 2021
@tklauser @aboch
Don't re-initialize or shadow package level var native to fix data race
4ef7bcb 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue #633.

Also don't re-declare local variables shadowing the global package-level
var.

Fixes #633

Signed-off-by: Tobias Klauser <[email protected]>
gkodali-zededa pushed a commit to gkodali-zededa/netlink that referenced this issue May 21, 2021
@tklauser @gkodali-zededa
Don't re-initialize or shadow package level var native to fix data race
f29a8ca 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue vishvananda#633.

Also don't re-declare local variables shadowing the global package-level
var.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
cyberys pushed a commit to cyberys/netlink that referenced this issue Apr 5, 2022
@tklauser
Don't re-initialize or shadow package level var native to fix data race
3b16161 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue vishvananda#633.

Also don't re-declare local variables shadowing the global package-level
var.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
cjmakes pushed a commit to cjmakes/netlink that referenced this issue Jun 29, 2022
@tklauser
Don't re-initialize or shadow package level var native to fix data race
59bb849 
The package level var native (holding the native endianness) is
initialized at package load time. Thus there is no need to re-initalize
it in functions using it, e.g. (*Handle).filterModify, parseU32Data,
parseFwData, parseBpfData and parseMatchAllData.

This fixes a data race between these functions and any read access of
var native, e.g. in LinkDeserialize as reported in issue vishvananda#633.

Also don't re-declare local variables shadowing the global package-level
var.

Fixes vishvananda#633

Signed-off-by: Tobias Klauser <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't re-initialize package level var native to fix data race tklauser/netlink
1 participant
@qmonnet