netdev - Re: [PATCHv2 bpf-next] samples/bpf: add xdp program on egress for xdp_redirect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201126141904.GA277949@localhost.localdomain>
Date:   Thu, 26 Nov 2020 22:19:04 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Tariq Toukan <tariqt@...lanox.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Subject: Re: [PATCHv2 bpf-next] samples/bpf: add xdp program on egress for
 xdp_redirect_map

On Thu, Nov 26, 2020 at 11:51:19AM +0100, Jesper Dangaard Brouer wrote:
> On Thu, 26 Nov 2020 16:43:25 +0800
> Hangbin Liu <liuhangbin@...il.com> wrote:
> 
> > Current sample test xdp_redirect_map only count pkts on ingress. But we
> > can't know whether the pkts are redirected or dropped. So add a counter
> > on egress interface so we could know how many pkts are redirect in fact.
> 
> This is not true.
> 
> The 2nd devmap XDP-prog will run in the same RX-context, so it doesn't
> tell us if the redirect was successful.  I looked up the code, and the
> 2nd XDP-prog is even allowed to run when the egress driver doesn't
> support the NDO to xmit (dev->netdev_ops->ndo_xdp_xmit), which is very
> misleading, if you place a output counter here.
> 
> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
> 
> 
> static inline int __xdp_enqueue(struct net_device *dev, struct xdp_buff *xdp,
> 			       struct net_device *dev_rx)
> {
> 	struct xdp_frame *xdpf;
> 	int err;
> 
> 	if (!dev->netdev_ops->ndo_xdp_xmit)
> 		return -EOPNOTSUPP;
> 

Err... You are right. I didn't read this part carefully.

Based on Toke's suggestion, maybe edit the src mac address to the egress
iface's mac address is a better example. WDYT?

And does it make any sense to run perf test for 2nd xdp program
for xdp_redirect_map?

Thanks
Hangbin


> 	err = xdp_ok_fwd_dev(dev, xdp->data_end - xdp->data);
> 	if (unlikely(err))
> 		return err;
> 
> 	xdpf = xdp_convert_buff_to_frame(xdp);
> 	if (unlikely(!xdpf))
> 		return -EOVERFLOW;
> 
> 	bq_enqueue(dev, xdpf, dev_rx);
> 	return 0;
> }
> 
> 
> int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
> 		    struct net_device *dev_rx)
> {
> 	struct net_device *dev = dst->dev;
> 
> 	if (dst->xdp_prog) {
> 		xdp = dev_map_run_prog(dev, xdp, dst->xdp_prog);
> 		if (!xdp)
> 			return 0;
> 	}
> 	return __xdp_enqueue(dev, xdp, dev_rx);
> }
>