Skip to content

Commit ed4096f

Browse files
committed
Address reviewer's comments.
Among which: added lint check for *either* license text or SPDX line in headers. Use the latter in all new files.
1 parent b4cacff commit ed4096f

18 files changed

Lines changed: 103 additions & 243 deletions

File tree

.golangci.yml

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,24 +41,29 @@ linters:
4141
msg: spell trust root certificate as trc / TRC
4242
goheader:
4343
values:
44-
regexp:
45-
copyright-lines: |-
46-
(Copyright 20[0-9][0-9] .*)(
47-
Copyright 20[0-9][0-9] .*)*
48-
template: |-
49-
{{copyright-lines}}
50-
51-
Licensed under the Apache License, Version 2.0 (the "License");
52-
you may not use this file except in compliance with the License.
53-
You may obtain a copy of the License at
44+
const:
45+
LICENSE_TEXT: |-
46+
Licensed under the Apache License, Version 2.0 \(the "License"\);
47+
you may not use this file except in compliance with the License.
48+
You may obtain a copy of the License at
5449
55-
http://www.apache.org/licenses/LICENSE-2.0
50+
http://www.apache.org/licenses/LICENSE-2.0
5651
57-
Unless required by applicable law or agreed to in writing, software
58-
distributed under the License is distributed on an "AS IS" BASIS,
59-
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
60-
See the License for the specific language governing permissions and
61-
limitations under the License.
52+
Unless required by applicable law or agreed to in writing, software
53+
distributed under the License is distributed on an "AS IS" BASIS,
54+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
55+
See the License for the specific language governing permissions and
56+
limitations under the License.
57+
SPDX_LINE: |-
58+
SPDX-License-Identifier: Apache-2.0
59+
regexp:
60+
copyright_lines: |-
61+
(Copyright 20[0-9][0-9] .*)
62+
(Copyright 20[0-9][0-9] .*)*
63+
license: |-
64+
({{SPDX_LINE}})|({{LICENSE_TEXT}})
65+
template: |-
66+
{{copyright_lines}}{{license}}
6267
lll:
6368
line-length: 100
6469
tab-width: 4

MODULE.bazel.lock

Lines changed: 12 additions & 41 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

acceptance/router_benchmark/brload/mmsg.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func newMpktSender(tp *afpacket.TPacket) *mpktSender {
6060

6161
// If we're going to send, we need to make sure we're not receiving our own stuff. The default
6262
// behaviour is less than clear. The loopback doesn't work with veth, but likely does with
63-
// else.
63+
// everything else.
6464
err = unix.SetsockoptInt(sender.fd, unix.SOL_PACKET, unix.PACKET_IGNORE_OUTGOING, 1)
6565
if err != nil {
6666
panic(err)

acceptance/router_benchmark/test.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -193,13 +193,12 @@ class RouterBMTest(base.TestBase, RouterBM):
193193
Pretend traffic is injected by brload's. See the test cases for details.
194194
"""
195195

196-
# TODO(jiceatscion): We construct intf_map during setup and we use it later, during
197-
# _run(). As a result, running setup, run, at teardown separately is difficult.
198-
# During run and teardown, we reconstruct the map without actually setup the
199-
# interfaces. This assumes that brload isn't being changed in-between, since the map is
200-
# based on the requirements that it outputs.
201-
debug_run: bool = DEBUG_RUN
196+
# We construct intf_map during setup and we use it later, during _run(). As a result, running
197+
# setup, run, and teardown separately is difficult. During run and teardown, we reconstruct the
198+
# map without actually setup the interfaces. This assumes that brload isn't being changed
199+
# in-between, since the map is based on the requirements that it outputs.
202200

201+
debug_run: bool = DEBUG_RUN
203202
router_cpus: list[int] = [0]
204203

205204
# Used by the RouterBM mixin:

private/underlay/ebpf/kfilter.c

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
// SPDX-License-Identifier: Apache-2.0
2-
//
31
// Copyright 2025 SCION Association
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
44

55
//go:build ignore
66

@@ -11,15 +11,18 @@
1111
#include <linux/udp.h>
1212
#include "bpf_helpers.h"
1313

14-
// This tells our bpf program which address/port(s) goes to the AF_PACKET socket
15-
// and therefore not to the kernel.
14+
// kfilter: the kernel-side filter. The purpose of this program is to prevent the traffic that goes
15+
// to the AF_PACKET socket from getting to the regular kernel networking stack as well. If it did,
16+
// the kernel would expand resources processing the traffic, generating ICMP responses AND sending
17+
// them!
1618
//
17-
// This is a set of (address/port) pairs. Ports must be in network byte order.
19+
// This is still not ideal because I have yet to find a way to dispatch traffic before it is cloned
20+
// for AF_PACKET handling but after it is turned into an SKB. To solve that problem we would need to
21+
// go to XDP.
1822
//
19-
// The same port numbers are used by sockfilter to perform the opposite filtering. We may have many
20-
// addrPorts to filter for a given interface. We could have several one-addrPort filters in series,
21-
// we would easily exceed the number of filters that can be attached to an interface (not
22-
// mentionning this would be inefficient). So we need a map with multiple ports.
23+
// This might not be as bad as it looks though: traffic is cloned via c-o-w and it might even not be
24+
// cloned until the AF_PACKET tap has made a drop/keep decision. The traffic that we keep is
25+
// definitely cloned; so... dear cow, a third swiss industry is now counting on you.
2326

2427
typedef struct {
2528
__u8 ip_addr[16];
@@ -28,28 +31,23 @@ typedef struct {
2831
__u8 padding; // just to make it clear what the real size of the struct is.
2932
} addrPort;
3033

34+
// k_map_flt tells our bpf program which address/port(s) go to the AF_PACKET socket and therefore
35+
// not to the kernel. Ports must be in network byte order.
36+
//
37+
// The same data is used by sockfilter to perform the opposite filtering. We may have many
38+
// pairs to filter for a given interface. We could have several one-addrPort filters in series,
39+
// but we would easily exceed the number of filters that can be attached to an interface (not
40+
// mentionning this would be inefficient). So we need a map with multiple pairs.
3141
struct {
3242
__uint(type, BPF_MAP_TYPE_HASH);
3343
__type(key, addrPort); // An IP address and a port number.
3444
__type(value, __u8); // Nothing. The map is just a set of keys.
3545
__uint(max_entries, 64);
3646
} k_map_flt SEC(".maps");
3747

38-
// The traffic that goes to the AF_PACKET socket, must not get to the regular kernel networking
39-
// stack; else it will expand resources processing it, generating ICMP responses AND sending them!
40-
// That is the purpose of this program.
41-
//
4248
// This is a very simple classifier type of filter: it looks at the packet's protocol and dest
4349
// port. If it is UDP and if the port is found in sock_map_filt, then the packet is dropped (because
4450
// the AF_PACKET socket will get and process it).
45-
//
46-
// This is not ideal because I have yet to find a way to dispatch traffic before it is cloned
47-
// for AF_PACKET handling but after it is turned into an SKB. To solve that problem we need to go to
48-
// XDP.
49-
//
50-
// This might not be as bad as it looks though: traffic is cloned via c-o-w and it might even not be
51-
// cloned until the AF_PACKET tap has made a drop/keep decision. The traffic that we keep is
52-
// definitely cloned; so... dear cow, a third swiss industry is now counting on you.
5351
SEC("tcx/ingress")
5452
int bpf_k_filter(struct __sk_buff *skb)
5553
{

private/underlay/ebpf/kfilter_lint.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,6 @@
1-
// SPDX-License-Identifier: Apache-2.0
2-
//
31
// Copyright 2025 SCION Association
42
//
5-
// Licensed under the Apache License, Version 2.0 (the "License");
6-
// you may not use this file except in compliance with the License.
7-
// You may obtain a copy of the License at
8-
//
9-
// http://www.apache.org/licenses/LICENSE-2.0
10-
//
11-
// Unless required by applicable law or agreed to in writing, software
12-
// distributed under the License is distributed on an "AS IS" BASIS,
13-
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14-
// See the License for the specific language governing permissions and
15-
// limitations under the License.
3+
// SPDX-License-Identifier: Apache-2.0
164

175
// Placeholder for generated code during lint.
186

private/underlay/ebpf/portfilter.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,6 @@
1-
// SPDX-License-Identifier: Apache-2.0
2-
//
31
// Copyright 2025 SCION Association
42
//
5-
// Licensed under the Apache License, Version 2.0 (the "License");
6-
// you may not use this file except in compliance with the License.
7-
// You may obtain a copy of the License at
8-
//
9-
// http://www.apache.org/licenses/LICENSE-2.0
10-
//
11-
// Unless required by applicable law or agreed to in writing, software
12-
// distributed under the License is distributed on an "AS IS" BASIS,
13-
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14-
// See the License for the specific language governing permissions and
15-
// limitations under the License.
3+
// SPDX-License-Identifier: Apache-2.0
164

175
package ebpf
186

private/underlay/ebpf/portfilter_test.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,6 @@
1-
// SPDX-License-Identifier: Apache-2.0
2-
//
31
// Copyright 2025 SCION Association
42
//
5-
// Licensed under the Apache License, Version 2.0 (the "License");
6-
// you may not use this file except in compliance with the License.
7-
// You may obtain a copy of the License at
8-
//
9-
// http://www.apache.org/licenses/LICENSE-2.0
10-
//
11-
// Unless required by applicable law or agreed to in writing, software
12-
// distributed under the License is distributed on an "AS IS" BASIS,
13-
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14-
// See the License for the specific language governing permissions and
15-
// limitations under the License.
3+
// SPDX-License-Identifier: Apache-2.0
164

175
package ebpf_test
186

0 commit comments

Comments
 (0)