On Tue, Apr 27, 2021 at 11:41 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
> > ...
> > +test_tc_peer_user
>
> can we make it into a reasonable test inside test_progs? that way it
> will be executed regularly
There doesn't seem to be any tests yet for redirect_peer in test_progs and I'd
like this test to live next to them. Would it make sense to rework
test_tc_redirect.sh
into the test_progs framework?
What's your thoughts on this Daniel?
> >
> > +SEC("src_ingress_l3") int tc_src_l3(struct __sk_buff *skb)
>
> please keep SEC() on separate line
>
> also, is this a BPF_PROG_TYPE_SCHED_CLS program? Can you usee libbpf's
> "classifier/src_ingress_l3" naming convention?
I'm following the conventions in that file. Should they all be
changed? Happy to do that.
> > +{
> > + __u16 proto = skb->protocol;
> > +
> > + if (bpf_skb_change_head(skb, ETH_HLEN, 0) != 0)
> > + return TC_ACT_SHOT;
> > +
> > + __u8 src_mac[] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55};
>
> we try to keep BPF code compliant with C89, so please move all the
> variable declaration into a single block at the top of a function
Will fix, thanks.
> > +void setns_by_name(char *name) {
>
>
> { should be on a new line
>
> please run scripts/checkpatch.pl on these files, it will point out
> trivial issues like this
Sorry about that. Will make sure I'll run it on all future submissions.
> > + int nsfd;
> > + char nspath[PATH_MAX];
> > +
> > + snprintf(nspath, sizeof(nspath), "%s/%s", "/var/run/netns", name);
> > + nsfd = open(nspath, O_RDONLY | O_CLOEXEC);
> > + if (nsfd < 0) {
> > + fprintf(stderr, "failed to open net namespace %s: %s\n", name, strerror(errno));
> > + exit(1);
> > + }
>
> here seems to be a mix of tabs and spaces
Thanks, will fix it and my editor's settings ;-).
