> >>> +
> >>> +    return 0;
> >>> +}
> >>> +#endif
> >>> +
> >>>   static int
> >>>   xsk_load_prog(struct netdev *netdev, const char *path,
> >>>                 struct bpf_object **pobj, int *prog_fd)
> >>>   {
> >>> +    struct netdev_linux *dev OVS_UNUSED = netdev_linux_cast(netdev);
> >>>       struct bpf_object_open_attr attr = {
> >>>           .prog_type = BPF_PROG_TYPE_XDP,
> >>>           .file = path,
> >>> @@ -298,6 +496,14 @@ xsk_load_prog(struct netdev *netdev, const char 
> >>> *path,
> >>>           goto err;
> >>>       }
> >>>
> >>> +#ifdef HAVE_XDP_OFFLOAD
> >>> +    if (!xdp_preload(netdev, obj)) {
> >>
> >> I forgot what's the purpose of doing xdp_preload, before we call
> >> bpf_object__load below.
> >> Does it just to check whether the flowtable_afxdp.o has every map we want?
> >> Or it does initialize something else.
>
> - Create map-in-maps as they cannot be automatically created.
> - Create xsks_map with the proper n_rxq parameter.
> - Use global output_map table instead of creating new one.
> All of them need to be done before bpf_object__load.

Now I understand, thanks!

>
> >>> +        VLOG_INFO("%s: Detected flowtable support in XDP program",
> >>> +                  netdev_get_name(netdev));
> >>> +        dev->has_xdp_flowtable = true;
> >>> +    }
> >>> +#endif
> >>> +
> >>>       if (bpf_object__load(obj)) {
> >>>           VLOG_ERR("%s: Can't load XDP program at '%s'",
> >>>                    netdev_get_name(netdev), path);
> >>> @@ -1297,7 +1503,17 @@ libbpf_print(enum libbpf_print_level level,
> >>>
> >>>   int netdev_afxdp_init(void)
> >>>   {
> >>> -    libbpf_set_print(libbpf_print);
> >>> +    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> >>> +
> >>> +    if (ovsthread_once_start(&once)) {
> >>> +        libbpf_set_print(libbpf_print);
> >>> +#ifdef HAVE_XDP_OFFLOAD
> >>> +        if (netdev_register_flow_api_provider(&netdev_offload_xdp)) {
> >>> +            VLOG_WARN("Failed to register XDP flow api provider");
> >>> +        }
> >>> +#endif
> >>> +        ovsthread_once_done(&once);
> >>> +    }
> >>>       return 0;
> >>>   }
> >>>
> >>> diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h
> >>> index e91cd102d..324152e8f 100644
> >>> --- a/lib/netdev-afxdp.h
> >>> +++ b/lib/netdev-afxdp.h
> >>> @@ -44,6 +44,7 @@ struct netdev_stats;
> >>>   struct smap;
> >>>   struct xdp_umem;
> >>>   struct xsk_socket_info;
> >>> +struct bpf_object;
> >>>
> >>>   int netdev_afxdp_rxq_construct(struct netdev_rxq *rxq_);
> >>>   void netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_);
> >>> @@ -70,6 +71,8 @@ int netdev_afxdp_get_custom_stats(const struct netdev 
> >>> *netdev,
> >>>   void free_afxdp_buf(struct dp_packet *p);
> >>>   int netdev_afxdp_reconfigure(struct netdev *netdev);
> >>>   void signal_remove_xdp(struct netdev *netdev);
> >>> +bool has_xdp_flowtable(struct netdev *netdev);
> >>> +struct bpf_object *get_xdp_object(struct netdev *netdev);
> >>>
> >>>   #else /* !HAVE_AF_XDP */
> >>>
> >> Thanks
> >> William
> >
> > btw, looking at lib/netdev-offload-xdp.c
> > the netdev_xdp_flow_get is not implemented.
> > Does that mean we will not detect flow that alread exists and always
> > replace with the new flow?
>
> IIUC flows are inserted when upcall happens. It does not use flow_get API and
> just insert flows.
> Without flow_get, we cannot use dpctl/get-flow or we cannot get flows stats.
> But implementing stats is not a trivial work as we need to prepare per-cpu 
> counters
> for flows so we don't slow down the XDP program. So I left it as a future 
> work.

I think we might want to consider adding it.
Because once people start to use XDP datapath, we will surely want to know
which flows are processed in XDP and which are not.

btw, I tested your patch again, and after ping between two namespaces,
then $ovs-ofctl dump-flows
show correct stats. I don't understand why without flow_get, it still shows
correct flow stats...

Thanks!
William
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to