Hello,
The principle looks good, here are some comments.
Samuel
Damien Zammit, le ven. 02 sept. 2022 09:06:33 +0000, a ecrit:
> diff --git a/pfinet/linux-src/net/ipv4/fib_hash.c
> b/pfinet/linux-src/net/ipv4/fib_hash.c
> index 074a36876..ca25377c9 100644
> --- a/pfinet/linux-src/net/ipv4/fib_hash.c
> +++ b/pfinet/linux-src/net/ipv4/fib_hash.c
> @@ -883,3 +883,87 @@ __initfunc(struct fib_table * fib_hash_init(int id))
> memset(tb->tb_data, 0, sizeof(struct fn_hash));
> return tb;
> }
> +
> +/* DZ: Could not fit this anywhere else, due to excessive struct defs in .c
> file */
Yes, that's fine.
> +static void
> +fib_node_get_route(int type, int dead, struct fib_info *fi, u32 prefix, u32
> mask, ifrtreq_t *r)
> +{
> + int len;
> + static unsigned type2flags[RTN_MAX+1] = {
> + 0, 0, 0, 0, 0, 0, 0, RTF_REJECT, RTF_REJECT, 0, 0, 0
Rather use simply [RTN_UNREACHABLE] = RTF_REJECT, [RTN_PROHIBIT] =
RTF_REJECT, the rest will already be zero.
[...]
> + if (fi && fi->fib_dev)
> + sprintf (r->ifname, "%s", fi->fib_dev->name);
This should be snprintf to propertly truncate the name rather than
overflow.
> +int
> +fn_hash_get_routes(struct fib_table *tb, ifrtreq_t *routes, int first, int
> count)
> +{
[...]
> + for (i=0; i < maxslot; i++, fp++)
> + {
> + for (f = *fp; f; f = f->fn_next)
> + {
> + if (++pos <= first)
> + continue;
> + fib_node_get_route(f->fn_type,
> + f->fn_state & FN_S_ZOMBIE,
> + FIB_INFO(f),
> + fz_prefix(f->fn_key, fz),
> + FZ_MASK(fz), routes);
> + routes++;
Take care of incoherent tab vs spaces.
> + if (++n >= count)
> + return n;
> + }
> + }
> + }
> + return n;
> +}
> diff --git a/pfinet/pfinet-ops.c b/pfinet/pfinet-ops.c
> index 5db669783..621721898 100644
> --- a/pfinet/pfinet-ops.c
> +++ b/pfinet/pfinet-ops.c
> @@ -22,6 +22,13 @@
>
> #include <linux/netdevice.h>
> #include <linux/notifier.h>
> +#include <linux/inetdevice.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/ip.h>
> +#include <net/route.h>
> +#include <net/sock.h>
> +#include <net/ip_fib.h>
> +#include <net/addrconf.h>
>
> #include "pfinet_S.h"
> #include <netinet/in.h>
> @@ -32,7 +39,8 @@
> #include <sys/mman.h>
>
> #include <sys/ioctl.h>
> -#include <net/if.h>
> +
> +#define MAX_ROUTES 255
>
> extern int dev_ifconf(char *arg);
>
> @@ -91,3 +99,77 @@ S_pfinet_siocgifconf (io_t port,
> pthread_mutex_unlock (&global_lock);
> return err;
> }
> +
> +int
> +get_routing_table(int start, int count, ifrtreq_t *routes)
> +{
> + struct fib_table *tb;
> +
> + if (!routes)
> + return 0;
> +
> + if ((tb = fib_get_table(RT_TABLE_MAIN)) == NULL)
> + return 0;
> +
> + return fn_hash_get_routes(tb, routes, start, count);
> +}
> +
> +
> +/* Return the routing table as a series of ifrtreq_t structs
> + in routes, but don't return more then AMOUNT number of them.
> + If AMOUNT is -1, we get the full table. */
> +error_t
> +S_pfinet_getroutes (io_t port,
> + vm_size_t amount,
> + data_t *routes,
> + mach_msg_type_number_t *len,
> + boolean_t *dealloc_data)
> +{
> + error_t err = 0;
> + ifrtreq_t rs[MAX_ROUTES];
> + int n;
> + ifrtreq_t *rtable;
> +
> + pthread_mutex_lock (&global_lock);
> +
> + if (dealloc_data)
> + *dealloc_data = FALSE;
> +
> + if (amount == (vm_size_t) -1)
> + {
> + /* Get all of them, and return the number we got. */
> + n = get_routing_table (0, MAX_ROUTES, rs);
> + amount = n;
> + }
> + else
> + n = amount;
> +
> + if (amount > 0)
> + {
> + /* Possibly allocate a new buffer. */
> + if (*len < amount * sizeof(ifrtreq_t))
> + rtable = (ifrtreq_t *) mmap (0, amount * sizeof(ifrtreq_t),
> PROT_READ|PROT_WRITE,
> + MAP_ANON, 0, 0);
Then *dealloc_data should be set to TRUE for mig to deallocate it.
> + memset(rtable, 0, n * sizeof(ifrtreq_t));
> + n = get_routing_table (0, n, rtable);
Rather only memset to the remainder of the array?
> + }
> +
> + if (rtable == MAP_FAILED)
> + {
> + err = ENOMEM;
Rather take errno?
> + *len = 0;
> + if ((char *)rtable != *routes)
> + munmap (rtable, amount * sizeof(ifrtreq_t));
Why unmapping? Here rtable is MAP_FAILED, munmaping it will just fail.
> + }
> + else
> + {
> + *len = n * sizeof(ifrtreq_t);
> + *routes = (char *)rtable;
> + }
> +
> + pthread_mutex_unlock (&global_lock);
> + return err;
> +}
> diff --git a/pfinet/route.h b/pfinet/route.h
> new file mode 100644
> index 000000000..8cb1b8a3f
> --- /dev/null
> +++ b/pfinet/route.h
> @@ -0,0 +1,26 @@
We need a copyright header.
> +#ifndef ROUTE_H_
> +#define ROUTE_H_
> +
> +#include <sys/socket.h>
> +#include <netinet/in.h>
> +#include <arpa/inet.h>
> +
> +#ifndef IFNAMSIZ
> +#define IFNAMSIZ 16
> +#endif
Can it happen that it's not defined? I'd rather not get a value randomly
defined here.
> +typedef struct ifrtreq {
> + char ifname[IFNAMSIZ];
> + in_addr_t rt_dest;
> + in_addr_t rt_mask;
> + in_addr_t rt_gateway;
> + int rt_flags;
> + int rt_metric;
> + int rt_mtu;
> + int rt_window;
> + int rt_irtt;
> + int rt_tos;
> + int rt_class;
> +} ifrtreq_t;
> +
> +#endif
