[PATCH] firewall3: remove unnecessary fw3_has_table

[Wenli Looi]

Hi Rui and Ansuel,

Can you take a look at this patch I sent a while ago for firewall3? I
think it is a better solution for the problem in kernel 5.15+ that is
identified here.

http://lists.openwrt.org/pipermail/openwrt-devel/2022-January/037534.html

Note that Ansuel's commit also seems to fix the problem with
LXC/LXD/Docker, because poking the table with fw3_ipt_open makes it
show up in ip_tables_names under Linux containers. However, as stated
in the commit, I don't think we need to check ip_tables_names at all?

Thanks!
Wenli


On Wed, Jun 9, 2021 at 9:51 PM Wenli Looi <wlooi at ucalgary.ca> wrote:
>
> Given that firewall3 already skips the table when fw3_ipt_open fails,
> there is no need for fw3_has_table.
>
> Furthermore, /proc/net/ip_tables_names is not reliable under linux
> containers (e.g. Docker/LXC/LXD). This patch will remove the need for
> existing hacks required for OpenWrt to run on those platforms.
>
> Signed-off-by: Wenli Looi <wlooi at ucalgary.ca>
> ---
> Additional comments:
>
> Under linux containers, I believe /proc/net/ip_tables_names does not
> contain the name of a table until it is accessed at least once.
>
> This patch makes firewall3 consistent with the iptables command, which
> fully works under linux containers and will output "Table does not
> exist" when iptc_init/ip6tc_init returns ENOENT.
>
> Examples of existing hacks required to run OpenWrt on those platforms:
>
> LXC: https://github.com/openwrt/openwrt/pull/2525
> LXD: https://github.com/cvmiller/openwrt-lxd/blob/bc09dc7ebf4f2904a9b717ed8a8a4065b5f8aaa5/init.sh#L67
> Docker: https://github.com/oofnikj/docker-openwrt/commit/a4f19bbbe1932e3b36690eb9ed75a273287120e3
>
> I've tested this patch on LXD and firewall3 appears to work without the
> above hack.
>
>  main.c  | 15 ---------------
>  utils.c |  9 ---------
>  utils.h |  2 --
>  3 files changed, 26 deletions(-)
>
> diff --git a/main.c b/main.c
> index 7ad00b4..7deb636 100644
> --- a/main.c
> +++ b/main.c
> @@ -195,9 +195,6 @@ stop(bool complete)
>
>                 for (table = FW3_TABLE_FILTER; table <= FW3_TABLE_RAW; table++)
>                 {
> -                       if (!fw3_has_table(family == FW3_FAMILY_V6, fw3_flag_names[table]))
> -                               continue;
> -
>                         if (!(handle = fw3_ipt_open(family, table)))
>                                 continue;
>
> @@ -268,9 +265,6 @@ start(void)
>
>                 for (table = FW3_TABLE_FILTER; table <= FW3_TABLE_RAW; table++)
>                 {
> -                       if (!fw3_has_table(family == FW3_FAMILY_V6, fw3_flag_names[table]))
> -                               continue;
> -
>                         if (!(handle = fw3_ipt_open(family, table)))
>                                 continue;
>
> @@ -339,9 +333,6 @@ reload(void)
>
>                 for (table = FW3_TABLE_FILTER; table <= FW3_TABLE_RAW; table++)
>                 {
> -                       if (!fw3_has_table(family == FW3_FAMILY_V6, fw3_flag_names[table]))
> -                               continue;
> -
>                         if (!(handle = fw3_ipt_open(family, table)))
>                                 continue;
>
> @@ -368,9 +359,6 @@ start:
>
>                 for (table = FW3_TABLE_FILTER; table <= FW3_TABLE_RAW; table++)
>                 {
> -                       if (!fw3_has_table(family == FW3_FAMILY_V6, fw3_flag_names[table]))
> -                               continue;
> -
>                         if (!(handle = fw3_ipt_open(family, table)))
>                                 continue;
>
> @@ -426,9 +414,6 @@ gc(void)
>
>                 for (table = FW3_TABLE_FILTER; table <= FW3_TABLE_RAW; table++)
>                 {
> -                       if (!fw3_has_table(family == FW3_FAMILY_V6, fw3_flag_names[table]))
> -                               continue;
> -
>                         if (!(handle = fw3_ipt_open(family, table)))
>                                 continue;
>
> diff --git a/utils.c b/utils.c
> index 17d5bf9..36897b0 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -339,15 +339,6 @@ file_contains(const char *path, const char *str)
>         return seen;
>  }
>
> -bool
> -fw3_has_table(const bool ipv6, const char *table)
> -{
> -       const char *path = ipv6
> -               ? "/proc/net/ip6_tables_names" : "/proc/net/ip_tables_names";
> -
> -       return file_contains(path, table);
> -}
> -
>  bool
>  fw3_has_target(const bool ipv6, const char *target)
>  {
> diff --git a/utils.h b/utils.h
> index 884907d..5b17a2d 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -102,8 +102,6 @@ void fw3_command_close(void);
>  void fw3_pr(const char *fmt, ...)
>         __attribute__ ((format (printf, 1, 2)));
>
> -bool fw3_has_table(const bool ipv6, const char *table);
> -
>  bool fw3_has_target(const bool ipv6, const char *target);
>
>  bool fw3_lock(void);
> --
> 2.25.1
>