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

Micro: Fix common.Swab16 with proper encoding/binary usage #285

Closed
2 tasks done
tgraf opened this issue Feb 10, 2017 · 2 comments
Closed
2 tasks done

Micro: Fix common.Swab16 with proper encoding/binary usage #285

tgraf opened this issue Feb 10, 2017 · 2 comments
Assignees
@aalemayhu
Labels
good-first-issue Good starting point for new developers, which requires minimal understanding of Cilium. kind/bug This is a bug in the Cilium logic. kind/enhancement This would improve or streamline existing functionality.
Projects
Micro Tasks

Comments

@tgraf
Copy link
Member

tgraf commented Feb 10, 2017 •
edited by aalemayhu

  • Create a new pkg/byteorder which replaces common.Swab*, based on encoding/binary after detecting the local byte order.
  • Convert all calls to common.Swab*

This is required for Cilium to function on ARM architectures
@tgraf tgraf created this issue from a note in Micro Tasks (TODO) Feb 10, 2017
@tgraf tgraf added the kind/enhancement This would improve or streamline existing functionality. label Feb 10, 2017
@tgraf tgraf added this to the 0.9 milestone Mar 18, 2017
@tgraf tgraf added kind/bug This is a bug in the Cilium logic. microtask labels Mar 18, 2017
@tgraf tgraf changed the title Revise byte order handling. Replace use of common.Swab16 with byte order detection and proper encoding/binary usage Micro: Fix common.Swab16 with proper encoding/binary usage Mar 20, 2017
@aalemayhu
Copy link
Contributor

The number of occurrences of common.Swab is:

Index File                      Line Content
    0 pkg/bpfdebug/debug.go      120 sport := common.Swab16(uint16(arg1 >> 16)) 
    1 pkg/bpfdebug/debug.go      121 dport := common.Swab16(uint16(arg1 & 0xFFFF)) 
    2 pkg/bpfdebug/debug.go      146 fmt.Printf("Local container found ifindex %d seclabel %d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
    3 pkg/bpfdebug/debug.go      154 fmt.Printf("CT entry found lifetime=%d, revnat=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
    4 pkg/bpfdebug/debug.go      158 fmt.Printf("CT created 2/2: %x revnat=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
    5 pkg/bpfdebug/debug.go      184 fmt.Printf("Master service lookup, addr.p4=%x key.dport=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
    6 pkg/bpfdebug/debug.go      188 fmt.Printf("Slave service lookup: slave=%d, dport=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
    7 pkg/bpfdebug/debug.go      190 fmt.Printf("Slave service lookup result: target.p4=%x port=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
    8 pkg/bpfdebug/debug.go      192 fmt.Printf("Reverse NAT lookup, index=%d\n", common.Swab16(uint16(n.Arg1))) 
    9 pkg/bpfdebug/debug.go      194 fmt.Printf("Performing reverse NAT, address.p4=%x port=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
   10 pkg/bpfdebug/debug.go      196 fmt.Printf("Master service lookup, addr=%x key.dport=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
   11 pkg/bpfdebug/debug.go      200 fmt.Printf("Slave service lookup result: target=%x port=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
   12 pkg/bpfdebug/debug.go      202 fmt.Printf("Performing reverse NAT, address=%x port=%d\n", n.Arg1, common.Swab16(uint16(n.Arg2))) 
   13 pkg/bpfdebug/debug.go      214 fmt.Printf("Resolved L4 policy to: %d / %d\n", common.Swab16(uint16(n.Arg1)), n.Arg2) 
   14 pkg/bpfdebug/debug.go      256 fmt.Printf("Packet to proxy port %d (Pre)\n", common.Swab16(uint16(n.Arg1))) 
   15 pkg/bpfdebug/debug.go      258 fmt.Printf("Packet to proxy port %d (Post)\n", common.Swab16(uint16(n.Arg1))) 
   16 pkg/endpoint/bpf.go         50 dport := common.Swab16(uint16(l4.Port)) 
   17 pkg/endpoint/bpf.go         60 redirect = common.Swab16(redirect) 
   18 pkg/endpoint/bpf.go        167 fmt.Fprintf(fw, "#define LXC_ID_NB %#x\n", common.Swab16(e.ID)) 
   19 pkg/endpoint/bpf.go        170 fmt.Fprintf(fw, "#define SECLABEL_NB %#x\n", common.Swab32(e.SecLabel.ID.Uint32())) 
   20 pkg/endpoint/bpf.go        174 fmt.Fprintf(fw, "#define SECLABEL_NB %#x\n", common.Swab32(invalid.Uint32())) 
   21 pkg/endpoint/bpf.go        192 fmt.Fprintf(fw, "{%#x,%#x},", common.Swab16(m.From), common.Swab16(m.To)) 
   22 pkg/maps/ctmap/ctmap.go    166 common.Swab16(value.revnat), 
   23 pkg/maps/ctmap/ctmap.go    167 common.Swab16(value.proxy_port)), 
   24 pkg/maps/lbmap/ipv4.go      70 n.Port = common.Swab16(n.Port) 
   25 pkg/maps/lbmap/ipv4.go     137 n.RevNat = common.Swab16(n.RevNat) 
   26 pkg/maps/lbmap/ipv4.go     138 n.Port = common.Swab16(n.Port) 
   27 pkg/maps/lbmap/ipv4.go     139 n.Weight = common.Swab16(n.Weight) 
   28 pkg/maps/lbmap/ipv4.go     202 n.Key = common.Swab16(n.Key) 
   29 pkg/maps/lbmap/ipv4.go     215 n.Port = common.Swab16(n.Port) 
   30 pkg/maps/lbmap/ipv6.go      77 n.Port = common.Swab16(n.Port) 
   31 pkg/maps/lbmap/ipv6.go     134 n.RevNat = common.Swab16(n.RevNat) 
   32 pkg/maps/lbmap/ipv6.go     135 n.Port = common.Swab16(n.Port) 
   33 pkg/maps/lbmap/ipv6.go     136 n.Weight = common.Swab16(n.Weight) 
   34 pkg/maps/lbmap/ipv6.go     195 n.Key = common.Swab16(n.Key) 
   35 pkg/maps/lbmap/ipv6.go     219 n.Port = common.Swab16(n.Port) 
   36 pkg/maps/lxcmap/lxcmap.go   83 return fmt.Sprintf("%d:%d", common.Swab16(pm.From), common.Swab16(pm.To)) 
   37 pkg/maps/lxcmap/lxcmap.go  120 common.Swab16(lxc.SecLabelID), 
   38 pkg/maps/lxcmap/lxcmap.go  149 SecLabelID: common.Swab16(uint16(ep.GetIdentity())), 
   39 pkg/maps/lxcmap/lxcmap.go  159 From: common.Swab16(pM.From), 
   40 pkg/maps/lxcmap/lxcmap.go  160 To:   common.Swab16(pM.To), 
   41 pkg/proxy/proxymap.go       75 n.SPort = common.Swab16(n.SPort) 
   42 pkg/proxy/proxymap.go       76 n.DPort = common.Swab16(n.DPort) 
   43 pkg/proxy/proxymap.go       86 n.OrigDPort = common.Swab16(n.OrigDPort)

@tgraf tgraf assigned aalemayhu and unassigned jrajahalme May 13, 2017
@aalemayhu
Copy link
Contributor

aalemayhu commented May 13, 2017 •
edited

@tgraf Not sure I understand the detection part, but is something like this(https://github.com/scanf/cilium/commit/1d547fc01eeeebc2f1c58c4b8c11cfd2fb384800) what you were thinking?

@aalemayhu aalemayhu mentioned this issue May 16, 2017
Merged
@tgraf tgraf modified the milestones: 0.10, 0.9 May 31, 2017
tgraf pushed a commit that referenced this issue Jul 11, 2017
@aalemayhu @tgraf
pkg: introduce new package for handling byteorder
0dfe617 
netlink is used for the byte order detection, which is already a
dependency available in the vendor directory. The package provides the
native byte order in the variable Native and wraps all byte order
specific reads and write call sites.

Related-to: #285 (Micro: Fix `common.Swab16` with proper `encoding/binary` usage)
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
tgraf pushed a commit that referenced this issue Jul 11, 2017
@aalemayhu @tgraf
bpf: don't call bpf_htonl for LXC_IPV4
4925039 
It's should already be in network byte order from Go side.

Related-to: #285 (Micro: Fix `common.Swab16` with proper `encoding/binary` usage)
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
tgraf pushed a commit that referenced this issue Jul 11, 2017
@aalemayhu @tgraf
byteorder: add documentation file for byteorder package
20d6e2f 
this should show a synopsis on godoc[0].

[0]: https://godoc.org/github.com/cilium/cilium

Closes: #285 (Micro: Fix common.Swab16 with proper encoding/binary usage)
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
amreshakim pushed a commit to amreshakim/cilium that referenced this issue Jul 13, 2017
@aalemayhu @amreshakim
pkg: introduce new package for handling byteorder
0ec249c 
netlink is used for the byte order detection, which is already a
dependency available in the vendor directory. The package provides the
native byte order in the variable Native and wraps all byte order
specific reads and write call sites.

Related-to: cilium#285 (Micro: Fix `common.Swab16` with proper `encoding/binary` usage)
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
amreshakim pushed a commit to amreshakim/cilium that referenced this issue Jul 13, 2017
@aalemayhu @amreshakim
bpf: don't call bpf_htonl for LXC_IPV4
58eeeb8 
It's should already be in network byte order from Go side.

Related-to: cilium#285 (Micro: Fix `common.Swab16` with proper `encoding/binary` usage)
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
amreshakim pushed a commit to amreshakim/cilium that referenced this issue Jul 13, 2017
@aalemayhu @amreshakim
byteorder: add documentation file for byteorder package
7309ba9 
this should show a synopsis on godoc[0].

[0]: https://godoc.org/github.com/cilium/cilium

Closes: cilium#285 (Micro: Fix common.Swab16 with proper encoding/binary usage)
Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
@pchaigno pchaigno added the good-first-issue Good starting point for new developers, which requires minimal understanding of Cilium. label Apr 2, 2020
michi-covalent pushed a commit to michi-covalent/cilium that referenced this issue May 30, 2023
@tklauser @michi-covalent
.golangci.yml: increase timeout
04fb769 
Static checks have timing out on the last few runs of cilium#285, increase the
timeout from the default 1m to 5m.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aalemayhu aalemayhu

Labels
good-first-issue Good starting point for new developers, which requires minimal understanding of Cilium. kind/bug This is a bug in the Cilium logic. kind/enhancement This would improve or streamline existing functionality.
Projects
No open projects
Micro Tasks
TODO
Milestone
No milestone
Development

No branches or pull requests
4 participants
@aalemayhu @tgraf @pchaigno @jrajahalme