from:"Alin Năstac"

Re: [PATCH odhcpd 2/2] router: always check ra_lifetime

2023-02-13 Thread Alin Năstac

On Mon, Feb 13, 2023 at 9:18 PM  wrote:
>
> @@ -495,13 +503,10 @@ static int send_router_advert(struct interface *iface, 
> const struct in6_addr *fr
> memcpy(addrs, iface->addr6, sizeof(*addrs) * 
> valid_addr_cnt);
>
> /* Check default route */
> -   if (iface->default_router) {
> -   default_route = true;
> -
> -   if (iface->default_router > 1)
> -   valid_prefix = true;
> -   } else if (parse_routes(addrs, valid_addr_cnt))
> -   default_route = true;
> +   if (!default_route) {
> +   if (parse_routes(addrs, valid_addr_cnt))
> +   default_route = true;
> +   }
> }
>
> if (invalid_addr_cnt) {

Patch looks fine to me, I have only one suggestion: merge these 2 if
statements into one.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH][netifd] system-linux: initialize ifreq struct before using it

2020-10-09 Thread Alin Năstac

Hi Hans,

I have the confirmation, this change fixed my problem.

Given the fact that my commit fixes an obvious programming error, I
don't think I need to put a detailed explanation in the commit
description.

BR,
Alin

On Thu, Oct 8, 2020 at 2:35 PM Alin Năstac  wrote:
>
> Hi Hans,
>
> The issue I have involved adding an external device to the lan bridge
> through add_device ubus call. Sometimes this operation fails to add
> the new bridge port with the following device debug traces:
> Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
> device_create_default(525): Create simple device 'wds1_1_2'
> Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
> device_init_virtual(478): Initialize device 'wds1_1_2'
> Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
> device_set_present(634): Network device 'wds1_1_2' is now present
> Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
> __device_add_user(710): Add user for device 'wds1_1_2', refcount=1
> Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]: device_claim(413):
> Claim Network device wds1_1_2, new active count: 1
> Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]: device_claim(432):
> claim Network device wds1_1_2 failed: -1
>
> I'm not sure it fixes the problem I experience (the jury is still out
> on that), but it is definitely caused by system_if_resolve() failure
> to retrieve the interface index. Looking at the handling of
> SIOCGIFINDEX in the kernel, it shouldn't matter if the rest of the
> fields are uninitialized, but a variable should always be initialized
> before its first use.
>
> BR,
> Alin
>
> On Thu, Oct 8, 2020 at 2:15 PM Hans Dedecker  wrote:
> >
> > Hi Alin,
> >
> > On Thu, Oct 8, 2020 at 1:31 PM Alin Nastac  wrote:
> > Could you add which issue this fixes ?
> >
> > Thx
> > Hans
> > >
> > > Signed-off-by: Alin Nastac 
> > > ---
> > >  system-linux.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/system-linux.c b/system-linux.c
> > > index 6778b1d..9188899 100644
> > > --- a/system-linux.c
> > > +++ b/system-linux.c
> > > @@ -904,6 +904,8 @@ failure:
> > >  int system_if_resolve(struct device *dev)
> > >  {
> > > struct ifreq ifr;
> > > +
> > > +   memset(, 0, sizeof(ifr));
> > > strncpy(ifr.ifr_name, dev->ifname, sizeof(ifr.ifr_name) - 1);
> > > if (!ioctl(sock_ioctl, SIOCGIFINDEX, ))
> > > return ifr.ifr_ifindex;
> > > --
> > > 2.7.4
> > >

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH][netifd] system-linux: initialize ifreq struct before using it

2020-10-08 Thread Alin Năstac

Hi Hans,

The issue I have involved adding an external device to the lan bridge
through add_device ubus call. Sometimes this operation fails to add
the new bridge port with the following device debug traces:
Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
device_create_default(525): Create simple device 'wds1_1_2'
Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
device_init_virtual(478): Initialize device 'wds1_1_2'
Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
device_set_present(634): Network device 'wds1_1_2' is now present
Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]:
__device_add_user(710): Add user for device 'wds1_1_2', refcount=1
Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]: device_claim(413):
Claim Network device wds1_1_2, new active count: 1
Wed Oct  7 04:22:57 2020 daemon.err netifd[2843]: device_claim(432):
claim Network device wds1_1_2 failed: -1

I'm not sure it fixes the problem I experience (the jury is still out
on that), but it is definitely caused by system_if_resolve() failure
to retrieve the interface index. Looking at the handling of
SIOCGIFINDEX in the kernel, it shouldn't matter if the rest of the
fields are uninitialized, but a variable should always be initialized
before its first use.

BR,
Alin

On Thu, Oct 8, 2020 at 2:15 PM Hans Dedecker  wrote:
>
> Hi Alin,
>
> On Thu, Oct 8, 2020 at 1:31 PM Alin Nastac  wrote:
> Could you add which issue this fixes ?
>
> Thx
> Hans
> >
> > Signed-off-by: Alin Nastac 
> > ---
> >  system-linux.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/system-linux.c b/system-linux.c
> > index 6778b1d..9188899 100644
> > --- a/system-linux.c
> > +++ b/system-linux.c
> > @@ -904,6 +904,8 @@ failure:
> >  int system_if_resolve(struct device *dev)
> >  {
> > struct ifreq ifr;
> > +
> > +   memset(, 0, sizeof(ifr));
> > strncpy(ifr.ifr_name, dev->ifname, sizeof(ifr.ifr_name) - 1);
> > if (!ioctl(sock_ioctl, SIOCGIFINDEX, ))
> > return ifr.ifr_ifindex;
> > --
> > 2.7.4
> >

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] iproute2: revert add libcap support, enabled in ip-full

2020-03-16 Thread Alin Năstac

On Sun, Mar 15, 2020 at 11:40 PM Mathias Kresin  wrote:
>
> 05/03/2020 23:29, Alin Năstac:
> > On Thu, Mar 5, 2020 at 8:34 PM Mathias Kresin  wrote:
> >>
> >> This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a.
> >
> > Not exactly a revert, since it keeps HAVE_CAP logic.
>
> Sure, I want to make sure that HAVE_CAP is really disabled.
>
> >> The libcap isn't as optional as the commit messages suggests. A hard
> >> dependency to the libcap package is added, which is only available in
> >> the external packages feed. Therefore it is impossible to package
> >> ip-full without having the external packages feed up and running, which
> >> is a regression to the former behaviour.
> >
> > The libcap support is by all means optional, otherwise iproute2 build
> > will fail when its missing.
>
> You describe exactly the issue I hit while building ip-full. During
> development I don't have any external/third-party feeds installed. And
> the OpenWrt base system shouldn't depend on external/third-party feeds.
> These feeds are an add on and by no means a requirement.
>
> > Besides, your commit actually removes this
> > dependency. If this is not a living proof of the facultative nature of
> > this dependency, I don't know what else is.
>
> Sorry, I don't get what you're trying to say.

I'm trying to say that libcap dependency is optional from iproute2 tarball pov.

I guess you were trying to say that openwrt +libcap dependency was not
optional, but I don't really understand why you think it isn't. ip
package has 2 variants (tiny and full) and I only enabled it on
ip-full variant.Since the variant selection is at user's discretion,
the ip openwrt package dependency of libcap *is* - by all means of the
word - optional.

> > One would argue that ip-full should correspond to the full fledged
> > version of the packet. If the dependency of an external package is the
> > issue, how about making a different variant with HAVE_CAP support? It
> > could be called ip-really-full (or ip-fullest).
>
> Try to get libcap into the OpenWrt base system and enable HAVE_CAP
> afterwards?

As I said in the previous message, my use case doesn't require libcap
functionality, so I have no problem with disabling HAVE_CAP in the
full variant, but from the semantic pov, full should really mean "all
functionality enabled". Maybe it would be wise to create a third
variant called standard that would be equivalent with the full variant
minus HAVE_CAP.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] iproute2: revert add libcap support, enabled in ip-full

2020-03-05 Thread Alin Năstac

On Thu, Mar 5, 2020 at 8:34 PM Mathias Kresin  wrote:
>
> This reverts commit a6da3f9ef746101b84a6f530f5a40de28341b69a.

Not exactly a revert, since it keeps HAVE_CAP logic.

> The libcap isn't as optional as the commit messages suggests. A hard
> dependency to the libcap package is added, which is only available in
> the external packages feed. Therefore it is impossible to package
> ip-full without having the external packages feed up and running, which
> is a regression to the former behaviour.

The libcap support is by all means optional, otherwise iproute2 build
will fail when its missing. Besides,your commit actually removes this
dependency. If this is not a living proof of the facultative nature of
this dependency, I don't know what else is.

One would argue that ip-full should correspond to the full fledged
version of the packet. If the dependency of an external package is the
issue, how about making a different variant with HAVE_CAP support? It
could be called ip-really-full (or ip-fullest).

Anyway, I don't really need libcap support in my iproute2. As long as
iproute2 continues to build successfully in the presence of libcap, it
would be fine for me.

> Signed-off-by: Mathias Kresin 
> ---
>  package/network/utils/iproute2/Makefile | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/package/network/utils/iproute2/Makefile 
> b/package/network/utils/iproute2/Makefile
> index 34b768a906..cff582283c 100644
> --- a/package/network/utils/iproute2/Makefile
> +++ b/package/network/utils/iproute2/Makefile
> @@ -47,7 +47,7 @@ $(call Package/iproute2/Default)
>   VARIANT:=full
>   PROVIDES:=ip
>   ALTERNATIVES:=300:/sbin/ip:/usr/libexec/ip-full
> - DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl +libcap
> + DEPENDS:=+libnl-tiny +libelf +(PACKAGE_devlink||PACKAGE_rdma):libmnl
>  endef
>
>  define Package/tc
> @@ -55,43 +55,43 @@ $(call Package/iproute2/Default)
>TITLE:=Traffic control utility
>VARIANT:=tc
>PROVIDES:=tc
> -  DEPENDS:=+kmod-sched-core +libxtables +libelf 
> +(PACKAGE_devlink||PACKAGE_rdma):libmnl +PACKAGE_ip-full:libcap
> +  DEPENDS:=+kmod-sched-core +libxtables +libelf 
> +(PACKAGE_devlink||PACKAGE_rdma):libmnl
>  endef
>
>  define Package/genl
>  $(call Package/iproute2/Default)
>TITLE:=General netlink utility frontend
> -  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap
> +  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf
>  endef
>
>  define Package/ip-bridge
>  $(call Package/iproute2/Default)
>TITLE:=Bridge configuration utility from iproute2
> -  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap
> +  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf
>  endef
>
>  define Package/ss
>  $(call Package/iproute2/Default)
>TITLE:=Socket statistics utility
> -  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap
> +  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf
>  endef
>
>  define Package/nstat
>  $(call Package/iproute2/Default)
>TITLE:=Network statistics utility
> -  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf +PACKAGE_ip-full:libcap
> +  DEPENDS:=+libnl-tiny +(PACKAGE_devlink||PACKAGE_rdma):libmnl 
> +(PACKAGE_tc||PACKAGE_ip-full):libelf
>  endef
>
>  define Package/devlink
>  $(call Package/iproute2/Default)
>TITLE:=Network devlink utility
> -  DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf 
> +PACKAGE_ip-full:libcap
> +  DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf
>  endef
>
>  define Package/rdma
>  $(call Package/iproute2/Default)
>TITLE:=Network rdma utility
> -  DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf 
> +PACKAGE_ip-full:libcap
> +  DEPENDS:=+libmnl +(PACKAGE_tc||PACKAGE_ip-full):libelf
>  endef
>
>  ifeq ($(BUILD_VARIANT),tiny)
> @@ -100,7 +100,7 @@ endif
>
>  ifeq ($(BUILD_VARIANT),full)
>HAVE_ELF:=y
> -  HAVE_CAP:=y
> +  HAVE_CAP:=n
>  endif
>
>  ifeq ($(BUILD_VARIANT),tc)
> --
> 2.17.1
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix route ownership to kernel when IPv6 address becomes deprecated

2020-02-05 Thread Alin Năstac

Since it has a different title, I presumed patchwork site will not
understand is the 2nd version of previous patch.

On Wed, Feb 5, 2020 at 2:39 PM Adrian Schmutzler
 wrote:
>
> Hi,
>
> please use a "v2" next time.
>
> Best
>
> Adrian
>
> > -Original Message-
> > From: Alin Năstac [mailto:alin.nas...@gmail.com]
> > Sent: Mittwoch, 5. Februar 2020 14:38
> > To: Adrian Schmutzler 
> > Cc: Hans Dedecker ; openwrt-devel  > de...@lists.openwrt.org>
> > Subject: Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix 
> > route
> > ownership to kernel when IPv6 address becomes deprecated
> >
> > Hi Adrian,
> >
> > This patch has been superseded by
> > https://patchwork.ozlabs.org/patch/1233845/
> >
> > Alin
> >
> > On Wed, Feb 5, 2020 at 1:56 PM Adrian Schmutzler
> >  wrote:
> > >
> > > Hi,
> > >
> > > works for me as well.
> > >
> > > However, I'd prefer a shorter commit title, e.g.
> > >
> > > interface-ip: transfer prefix route ownership for deprecated ipv6addr to 
> > > kernel
> > >
> > > Best
> > >
> > > Adrian
> > >
> > > > -Original Message-
> > > > From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org] On
> > > > Behalf Of Alin Nastac
> > > > Sent: Mittwoch, 5. Februar 2020 09:34
> > > > To: Hans Dedecker 
> > > > Cc: openwrt-devel 
> > > > Subject: Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer 
> > > > prefix
> > > route
> > > > ownership to kernel when IPv6 address becomes deprecated
> > > >
> > > > Hi Hans,
> > > >
> > > > On Tue, Feb 4, 2020 at 10:49 PM Hans Dedecker 
> > wrote:
> > > > >
> > > > > Hi Alin,
> > > > > On Mon, Feb 3, 2020 at 4:27 PM Alin Nastac 
> > wrote:
> > > > > >
> > > > > > From: Alin Nastac 
> > > > > >
> > > > > > When netifd manages the prefix route directly, it will remove it
> > > > > > the moment prefix gets deprecated. This will make it impossible
> > > > > > for the target to send ICMPv6 errors back to LAN devices still
> > > > > > using the deprecated prefix, thus breaking the L-14 requirement
> > > > > > of RFC 7084.
> > > > > >
> > > > > > Signed-off-by: Alin Nastac 
> > > > > The patch fails to apply with the following error message :
> > > > >
> > > > > Applying: interface-ip: transfer prefix route ownership to kernel when
> > > > > IPv6 address becomes deprecated
> > > > > error: sha1 information is lacking or useless (interface-ip.c).
> > > > > error: could not build fake ancestor
> > > > > Patch failed at 0001 interface-ip: transfer prefix route ownership to
> > > > > kernel when IPv6 address becomes deprecated
> > > > >
> > > > > > route.addr = addr.addr;
> > > > > > }
> > > > > >
> > > > > > +   addr.flags |= DEVADDR_OFFLINK;
> > > > > > if (system_add_address(l3_downlink, ))
> > > > > > return;
> > > > > >
> > > > > > --
> > > > > > 2.7.4
> > > > > >
> > > >
> > > > I've downloaded the patch from
> > > > https://patchwork.ozlabs.org/patch/1232885/ and applied it
> > > > successfully with a git am command line:
> > > >
> > > > nastaca@cplx1037:/usr/localdisk/openwrt/netifd$ git am
> > > > ~/Downloads/OpenWrt-Devel-netifd-interface-ip-transfer-prefix-route-
> > > > ownership-to-kernel-when-IPv6-address-becomes
> > > > -deprecated.patch
> > > > Applying: interface-ip: transfer prefix route ownership to kernel when
> > > > IPv6 address becomes deprecated
> > > >
> > > > Alin
> > > >
> > > > ___
> > > > openwrt-devel mailing list
> > > > openwrt-devel@lists.openwrt.org
> > > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> > >
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix route ownership to kernel when IPv6 address becomes deprecated

2020-02-05 Thread Alin Năstac

Hi Adrian,

This patch has been superseded by https://patchwork.ozlabs.org/patch/1233845/

Alin

On Wed, Feb 5, 2020 at 1:56 PM Adrian Schmutzler
 wrote:
>
> Hi,
>
> works for me as well.
>
> However, I'd prefer a shorter commit title, e.g.
>
> interface-ip: transfer prefix route ownership for deprecated ipv6addr to 
> kernel
>
> Best
>
> Adrian
>
> > -Original Message-
> > From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org] On
> > Behalf Of Alin Nastac
> > Sent: Mittwoch, 5. Februar 2020 09:34
> > To: Hans Dedecker 
> > Cc: openwrt-devel 
> > Subject: Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix
> route
> > ownership to kernel when IPv6 address becomes deprecated
> >
> > Hi Hans,
> >
> > On Tue, Feb 4, 2020 at 10:49 PM Hans Dedecker  wrote:
> > >
> > > Hi Alin,
> > > On Mon, Feb 3, 2020 at 4:27 PM Alin Nastac  wrote:
> > > >
> > > > From: Alin Nastac 
> > > >
> > > > When netifd manages the prefix route directly, it will remove it
> > > > the moment prefix gets deprecated. This will make it impossible
> > > > for the target to send ICMPv6 errors back to LAN devices still
> > > > using the deprecated prefix, thus breaking the L-14 requirement
> > > > of RFC 7084.
> > > >
> > > > Signed-off-by: Alin Nastac 
> > > The patch fails to apply with the following error message :
> > >
> > > Applying: interface-ip: transfer prefix route ownership to kernel when
> > > IPv6 address becomes deprecated
> > > error: sha1 information is lacking or useless (interface-ip.c).
> > > error: could not build fake ancestor
> > > Patch failed at 0001 interface-ip: transfer prefix route ownership to
> > > kernel when IPv6 address becomes deprecated
> > >
> > > > route.addr = addr.addr;
> > > > }
> > > >
> > > > +   addr.flags |= DEVADDR_OFFLINK;
> > > > if (system_add_address(l3_downlink, ))
> > > > return;
> > > >
> > > > --
> > > > 2.7.4
> > > >
> >
> > I've downloaded the patch from
> > https://patchwork.ozlabs.org/patch/1232885/ and applied it
> > successfully with a git am command line:
> >
> > nastaca@cplx1037:/usr/localdisk/openwrt/netifd$ git am
> > ~/Downloads/OpenWrt-Devel-netifd-interface-ip-transfer-prefix-route-
> > ownership-to-kernel-when-IPv6-address-becomes
> > -deprecated.patch
> > Applying: interface-ip: transfer prefix route ownership to kernel when
> > IPv6 address becomes deprecated
> >
> > Alin
> >
> > ___
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix route ownership to kernel when IPv6 address becomes deprecated

2020-02-05 Thread Alin Năstac

Hi Hans,

On Tue, Feb 4, 2020 at 10:49 PM Hans Dedecker  wrote:
>
> Hi Alin,
> On Mon, Feb 3, 2020 at 4:27 PM Alin Nastac  wrote:
> >
> > From: Alin Nastac 
> >
> > When netifd manages the prefix route directly, it will remove it
> > the moment prefix gets deprecated. This will make it impossible
> > for the target to send ICMPv6 errors back to LAN devices still
> > using the deprecated prefix, thus breaking the L-14 requirement
> > of RFC 7084.
> >
> > Signed-off-by: Alin Nastac 
> The patch fails to apply with the following error message :
>
> Applying: interface-ip: transfer prefix route ownership to kernel when
> IPv6 address becomes deprecated
> error: sha1 information is lacking or useless (interface-ip.c).
> error: could not build fake ancestor
> Patch failed at 0001 interface-ip: transfer prefix route ownership to
> kernel when IPv6 address becomes deprecated
>
> > route.addr = addr.addr;
> > }
> >
> > +   addr.flags |= DEVADDR_OFFLINK;
> > if (system_add_address(l3_downlink, ))
> > return;
> >
> > --
> > 2.7.4
> >

I've downloaded the patch from
https://patchwork.ozlabs.org/patch/1232885/ and applied it
successfully with a git am command line:

nastaca@cplx1037:/usr/localdisk/openwrt/netifd$ git am
~/Downloads/OpenWrt-Devel-netifd-interface-ip-transfer-prefix-route-ownership-to-kernel-when-IPv6-address-becomes
-deprecated.patch
Applying: interface-ip: transfer prefix route ownership to kernel when
IPv6 address becomes deprecated

Alin

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] interface: add IPv6 addresses without IFA_F_NOPREFIXROUTE

2020-02-03 Thread Alin Năstac

On Wed, Dec 18, 2019 at 11:44 AM Alin Nastac  wrote:
>
> When netifd manages the prefix route directly, it will remove it
> the moment prefix gets deprecated. This will make it impossible
> for the target to send ICMPv6 errors back to LAN devices still
> using the deprecated prefix, thus breaking the L-14 requirement
> of RFC 7084.
>
> Signed-off-by: Alin Nastac 
> ---
>  interface-ip.c | 18 +-
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/interface-ip.c b/interface-ip.c
> index c159e09..0958fcb 100644
> --- a/interface-ip.c
> +++ b/interface-ip.c
> @@ -905,20 +905,14 @@ interface_set_prefix_address(struct 
> device_prefix_assignment *assignment,
> struct device *l3_downlink = iface->l3_dev.dev;
>
> struct device_addr addr;
> -   struct device_route route;
> memset(, 0, sizeof(addr));
> -   memset(, 0, sizeof(route));
>
> addr.addr.in6 = assignment->addr;
> addr.mask = assignment->length;
> -   addr.flags = DEVADDR_INET6 | DEVADDR_OFFLINK;
> +   addr.flags = DEVADDR_INET6;
> addr.preferred_until = prefix->preferred_until;
> addr.valid_until = prefix->valid_until;
>
> -   route.flags = DEVADDR_INET6;
> -   route.mask = addr.mask < 64 ? 64 : addr.mask;
> -   route.addr = addr.addr;
> -
> if (!add && assignment->enabled) {
> time_t now = system_get_rtime();
>
> @@ -939,10 +933,6 @@ interface_set_prefix_address(struct 
> device_prefix_assignment *assignment,
> addr.mask, 0, iface, 
> "unreachable", true);
> }
>
> -   clear_if_addr(, route.mask);
> -   interface_set_route_info(iface, );
> -
> -   system_del_route(l3_downlink, );
> system_add_address(l3_downlink, );
>
> assignment->addr = in6addr_any;
> @@ -955,7 +945,6 @@ interface_set_prefix_address(struct 
> device_prefix_assignment *assignment,
> return;
>
> assignment->addr = addr.addr.in6;
> -   route.addr = addr.addr;
> }
>
> if (system_add_address(l3_downlink, ))
> @@ -976,11 +965,6 @@ interface_set_prefix_address(struct 
> device_prefix_assignment *assignment,
> }
> }
>
> -   clear_if_addr(, route.mask);
> -   interface_set_route_info(iface, );
> -
> -   system_add_route(l3_downlink, );
> -
> if (uplink && uplink->l3_dev.dev && 
> !(l3_downlink->settings.flags & DEV_OPT_MTU6)) {
> int mtu = system_update_ipv6_mtu(uplink->l3_dev.dev, 
> 0);
> int mtu_old = system_update_ipv6_mtu(l3_downlink, 0);
> --
> 2.7.4
>

This patch has been superseded by https://patchwork.ozlabs.org/patch/1232885/ .

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH v2] fstools: add a hook before mounting the overlay

2019-10-31 Thread Alin Năstac

On Thu, Oct 24, 2019 at 11:24 PM Karl Palsson  wrote:
>
>
> Alin Nastac  wrote:
> > From: Alin Nastac 
> >
> > Scripts located in the directory /etc/mount_root.d will be
> > executed before mounting the overlay. It can be used to
> > implement configuration merges between old & new setup after
> > doing sysupgrade.
>
> >From the name of the directory it's unclear when these would be
> executed. (You say pre, it's what you needed, but what if I
> wanted afterwards? where would they go?)
>
> Perhaps these could be made compatible with the hotplug.d
> scripts, where they have environment variables passed to them
> with the ACTION (pre/post) and so on. See
> https://openwrt.org/docs/guide-user/base-system/hotplug
>
> You could even just have another directory under /etc/hotplug.d ?

The only action handled in my scripts is "pre-overlay-activation" and
I used it to merge parts of the old config into the new overlay file
system. If community finds this useful, I can move the scripts to
/etc/hotplug.d/overlay and run the equivalent of the shell command
   ACTION=activate /sbin/hotplug-call overlay

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] fstools: add a hook before mounting the overlay

2019-10-10 Thread Alin Năstac

On Wed, Oct 9, 2019 at 4:52 PM Alin Năstac  wrote:
>
> On Wed, Oct 9, 2019 at 4:41 PM John Crispin  wrote:
> >
> >
> > On 09/10/2019 16:34, Alin Năstac wrote:
> > > On Wed, Oct 9, 2019 at 2:59 PM John Crispin  wrote:
> > >>
> > >> On 09/10/2019 14:41, Alin Nastac wrote:
> > >>> Scripts located in the directory /lib/mount_root will be executed
> > >>> before mounting the overlay.
> > >>>
> > >>> Signed-off-by: Alin Nastac 
> > >> Hi,
> > >>
> > >> should it not be /etc/mount_root.d/ ? what do you need this for if I may
> > >> ask ?
> > >>
> > >> further comments inline ...
> > >>
> > >>   John
> > >>
> > > Hi John,
> > >
> > > My target is dual bank and I need to copy parts of the customization
> > > from the old bank after upgrade.
> >
> > please dont remove the CC tot he mailing list
> Sorry, I pushed the wrong reply button.
>
> > I dont understamd this part, should sysupgrade not be able to handle
> > this for you ?
>
> Well, it is not that straightforward as saving & restoring the old
> configuration files. Only parts of the UCI configuration must be
> migrated. For instance, UCI option a.b.c must be copied from the old
> image, but a.b.d must be reset to the value found in the new image.
>

Is this use case a good enough reason to implement this feature?

> > > To resume your observations:
> > >   - scripts must be relocated to /etc/mount_root.d/
> > >   - use runqueue API
> > >   - implement the necessary functions in overlay.c
> > > Would that be OK? If so, I will be back with a 2nd version of this patch.
> >
> > I would first like to understand the use-case
> >
> >  John

I've tried to use runqueue API... This API is designed to be used in
conjunction with uloop, which is designed to be used within daemons.
mount_root is not a daemon and it does not have a main loop, so IMHO
it does not fit well in this place.

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] fstools: add a hook before mounting the overlay

2019-10-09 Thread Alin Năstac

On Wed, Oct 9, 2019 at 4:41 PM John Crispin  wrote:
>
>
> On 09/10/2019 16:34, Alin Năstac wrote:
> > On Wed, Oct 9, 2019 at 2:59 PM John Crispin  wrote:
> >>
> >> On 09/10/2019 14:41, Alin Nastac wrote:
> >>> Scripts located in the directory /lib/mount_root will be executed
> >>> before mounting the overlay.
> >>>
> >>> Signed-off-by: Alin Nastac 
> >> Hi,
> >>
> >> should it not be /etc/mount_root.d/ ? what do you need this for if I may
> >> ask ?
> >>
> >> further comments inline ...
> >>
> >>   John
> >>
> > Hi John,
> >
> > My target is dual bank and I need to copy parts of the customization
> > from the old bank after upgrade.
>
> please dont remove the CC tot he mailing list
Sorry, I pushed the wrong reply button.

> I dont understamd this part, should sysupgrade not be able to handle
> this for you ?

Well, it is not that straightforward as saving & restoring the old
configuration files. Only parts of the UCI configuration must be
migrated. For instance, UCI option a.b.c must be copied from the old
image, but a.b.d must be reset to the value found in the new image.

> > To resume your observations:
> >   - scripts must be relocated to /etc/mount_root.d/
> >   - use runqueue API
> >   - implement the necessary functions in overlay.c
> > Would that be OK? If so, I will be back with a 2nd version of this patch.
>
> I would first like to understand the use-case
>
>  John
>
> >
> > Alin

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] firewall3: make reject types selectable by user

2018-07-03 Thread Alin Năstac

On Tue, Jul 3, 2018 at 11:32 PM Philip Prindeville
 wrote:
> > On Jul 3, 2018, at 3:22 PM, Alin Năstac  wrote:
> >
> > On Tue, Jul 3, 2018 at 6:39 PM Philip Prindeville
> >  wrote:
> >>
> >> Aren’t all inbound SYNs unsolicited by definition? Is there a danger of 
> >> reflection attacks?
> >
> > Not all inbound SYNs are unsolicited. Take for instance active mode
> > FTP transfers where the client resides on the LAN . In this case the
> > FTP data connection is initiated from the WAN, but it is solicited by
> > the FTP control connection initiated from the LAN.
> >
> > I don't think it matters that much what error code firewall returns
> > for these unsolicited  inbound SYNs, but this RFC makes
> > adm-prohibitited code a must.
>
> I would have thought that dropping them would be better, since it avoids 
> reflection attacks.

Whether you want to silently drop or reject unauthorized connection
attempts is a matter of local policy.

Besides, in order for a reflection attack against your LAN to succeed,
the source IP address of rejected packets must be part of the LAN
prefix. This can be easily prevented, either by enabling rpfilter or
just by adding a firewall rule when the LAN prefix is statically
allocated (the usual IPv4 case).

> >>> On Jul 2, 2018, at 9:29 AM, Alin Nastac  wrote:
> >>>
> >>> From: Alin Nastac 
> >>>
> >>> RFC 6092 recommends in section 3.3.1 that an IPv6 CPE must respond to
> >>> unsolicited inbound SYNs with an ICMPv6 Destination Unreachable error
> >>> code 1 (Communication with destination administratively prohibited).
> >>>
> >>> Signed-off-by: Alin Nastac 
> >>> ---
> >>> defaults.c | 21 -
> >>> options.h  |  2 ++
> >>> 2 files changed, 18 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/defaults.c b/defaults.c
> >>> index 11fbf0d..6565ca2 100644
> >>> --- a/defaults.c
> >>> +++ b/defaults.c
> >>> @@ -41,6 +41,8 @@ const struct fw3_option fw3_flag_opts[] = {
> >>>   FW3_OPT("output",  target,   defaults, policy_output),
> >>>
> >>>   FW3_OPT("drop_invalid",bool, defaults, drop_invalid),
> >>> +FW3_OPT("tcp_reset_rejects",   bool, defaults, 
> >>> tcp_reset_rejects),
> >>> +FW3_OPT("admin_prohib_rejects",bool, defaults, 
> >>> admin_prohib_rejects),
> >>>
> >>>   FW3_OPT("syn_flood",   bool, defaults, syn_flood),
> >>>   FW3_OPT("synflood_protect",bool, defaults, syn_flood),
> >>> @@ -113,6 +115,7 @@ fw3_load_defaults(struct fw3_state *state, struct 
> >>> uci_package *p)
> >>>
> >>>   defs->syn_flood_rate.rate  = 25;
> >>>   defs->syn_flood_rate.burst = 50;
> >>> +defs->tcp_reset_rejects= true;
> >>>   defs->tcp_syncookies   = true;
> >>>   defs->tcp_window_scaling   = true;
> >>>   defs->custom_chains= true;
> >>> @@ -276,14 +279,22 @@ fw3_print_default_head_rules(struct fw3_ipt_handle 
> >>> *handle,
> >>>   fw3_ipt_rule_append(r, "INPUT");
> >>>   }
> >>>
> >>> -r = fw3_ipt_rule_create(handle, , NULL, NULL, NULL, NULL);
> >>> -fw3_ipt_rule_target(r, "REJECT");
> >>> -fw3_ipt_rule_addarg(r, false, "--reject-with", "tcp-reset");
> >>> -fw3_ipt_rule_append(r, "reject");
> >>> +if (defs->tcp_reset_rejects)
> >>> +{
> >>> +r = fw3_ipt_rule_create(handle, , NULL, NULL, NULL, 
> >>> NULL);
> >>> +fw3_ipt_rule_target(r, "REJECT");
> >>> +fw3_ipt_rule_addarg(r, false, "--reject-with", "tcp-reset");
> >>> +fw3_ipt_rule_append(r, "reject");
> >>> +}
> >>>
> >>>   r = fw3_ipt_rule_new(handle);
> >>>   fw3_ipt_rule_target(r, "REJECT");
> >>> -fw3_ipt_rule_addarg(r, false, "--reject-with", "port-unreach");
> >>> +fw3_ipt_rule_addarg(r, false, "--reject-with",
> >>> +defs->admin_prohib_rejects ?
> >>> +(handle->family == FW3_FAMILY_V6 ?
> >>> +"adm-prohibited" :
> >>> +"admin-prohib") :
> >>> +"port-unreach");
> >>>   fw3_ipt_rule_append(r, "reject");
> >>>
> >>>   break;
> >>> diff --git a/options.h b/options.h
> >>> index 08fecf6..e3ba99c 100644
> >>> --- a/options.h
> >>> +++ b/options.h
> >>> @@ -276,6 +276,8 @@ struct fw3_defaults
> >>>   enum fw3_flag policy_forward;
> >>>
> >>>   bool drop_invalid;
> >>> +bool tcp_reset_rejects;
> >>> +bool admin_prohib_rejects;
> >>>
> >>>   bool syn_flood;
> >>>   struct fw3_limit syn_flood_rate;
> >>> --
> >>> 2.7.4
> >>>
> >>>
> >>> ___
> >>> openwrt-devel mailing list
> >>> openwrt-devel@lists.openwrt.org
> >>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> >>
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] firewall3: make reject types selectable by user

2018-07-03 Thread Alin Năstac

On Tue, Jul 3, 2018 at 6:39 PM Philip Prindeville
 wrote:
>
> Aren’t all inbound SYNs unsolicited by definition? Is there a danger of 
> reflection attacks?

Not all inbound SYNs are unsolicited. Take for instance active mode
FTP transfers where the client resides on the LAN . In this case the
FTP data connection is initiated from the WAN, but it is solicited by
the FTP control connection initiated from the LAN.

I don't think it matters that much what error code firewall returns
for these unsolicited  inbound SYNs, but this RFC makes
adm-prohibitited code a must.

> Sent from my iPhone
> > On Jul 2, 2018, at 9:29 AM, Alin Nastac  wrote:
> >
> > From: Alin Nastac 
> >
> > RFC 6092 recommends in section 3.3.1 that an IPv6 CPE must respond to
> > unsolicited inbound SYNs with an ICMPv6 Destination Unreachable error
> > code 1 (Communication with destination administratively prohibited).
> >
> > Signed-off-by: Alin Nastac 
> > ---
> > defaults.c | 21 -
> > options.h  |  2 ++
> > 2 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/defaults.c b/defaults.c
> > index 11fbf0d..6565ca2 100644
> > --- a/defaults.c
> > +++ b/defaults.c
> > @@ -41,6 +41,8 @@ const struct fw3_option fw3_flag_opts[] = {
> >FW3_OPT("output",  target,   defaults, policy_output),
> >
> >FW3_OPT("drop_invalid",bool, defaults, drop_invalid),
> > +FW3_OPT("tcp_reset_rejects",   bool, defaults, tcp_reset_rejects),
> > +FW3_OPT("admin_prohib_rejects",bool, defaults, 
> > admin_prohib_rejects),
> >
> >FW3_OPT("syn_flood",   bool, defaults, syn_flood),
> >FW3_OPT("synflood_protect",bool, defaults, syn_flood),
> > @@ -113,6 +115,7 @@ fw3_load_defaults(struct fw3_state *state, struct 
> > uci_package *p)
> >
> >defs->syn_flood_rate.rate  = 25;
> >defs->syn_flood_rate.burst = 50;
> > +defs->tcp_reset_rejects= true;
> >defs->tcp_syncookies   = true;
> >defs->tcp_window_scaling   = true;
> >defs->custom_chains= true;
> > @@ -276,14 +279,22 @@ fw3_print_default_head_rules(struct fw3_ipt_handle 
> > *handle,
> >fw3_ipt_rule_append(r, "INPUT");
> >}
> >
> > -r = fw3_ipt_rule_create(handle, , NULL, NULL, NULL, NULL);
> > -fw3_ipt_rule_target(r, "REJECT");
> > -fw3_ipt_rule_addarg(r, false, "--reject-with", "tcp-reset");
> > -fw3_ipt_rule_append(r, "reject");
> > +if (defs->tcp_reset_rejects)
> > +{
> > +r = fw3_ipt_rule_create(handle, , NULL, NULL, NULL, NULL);
> > +fw3_ipt_rule_target(r, "REJECT");
> > +fw3_ipt_rule_addarg(r, false, "--reject-with", "tcp-reset");
> > +fw3_ipt_rule_append(r, "reject");
> > +}
> >
> >r = fw3_ipt_rule_new(handle);
> >fw3_ipt_rule_target(r, "REJECT");
> > -fw3_ipt_rule_addarg(r, false, "--reject-with", "port-unreach");
> > +fw3_ipt_rule_addarg(r, false, "--reject-with",
> > +defs->admin_prohib_rejects ?
> > +(handle->family == FW3_FAMILY_V6 ?
> > +"adm-prohibited" :
> > +"admin-prohib") :
> > +"port-unreach");
> >fw3_ipt_rule_append(r, "reject");
> >
> >break;
> > diff --git a/options.h b/options.h
> > index 08fecf6..e3ba99c 100644
> > --- a/options.h
> > +++ b/options.h
> > @@ -276,6 +276,8 @@ struct fw3_defaults
> >enum fw3_flag policy_forward;
> >
> >bool drop_invalid;
> > +bool tcp_reset_rejects;
> > +bool admin_prohib_rejects;
> >
> >bool syn_flood;
> >struct fw3_limit syn_flood_rate;
> > --
> > 2.7.4
> >
> >
> > ___
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] procd: service gets deleted when its last instance is freed

2017-02-24 Thread Alin Năstac

Hi John,

On Fri, Feb 24, 2017 at 10:53 AM, John Crispin  wrote:
> can you write a little more info as to why this is needed and what
> scenario this fixes/changes ?

1) root@OpenWrt:~# uci show system.ntp
system.ntp=timeserver
system.ntp.enable_server='0'
system.ntp.use_dhcp='1'
system.ntp.dhcp_interface='wan'
2) root@OpenWrt:~# uci show network.wan
network.wan=interface
network.wan.proto='dhcp'
network.wan.ifname='eth4'
network.wan.reqopts='1 3 6 15 33 42 51 121 249'
3) restart wan interface using ifup command
4) sysntpd service will be down afterwards although dhcp lease has an option 42

Because service is deleted when last instance gets freed, its triggers
will also released. Without these triggers in place, sysntpd will not
be reloaded when a new dhcp lease containing option 42 will be
received.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [LEDE-DEV] [PATCH] procd: stop service using SIGKILL if SIGTERM failed to do so

2017-02-09 Thread Alin Năstac

On Thu, Feb 9, 2017 at 11:54 AM, John Crispin  wrote:
> Hi,
>
> i know that someone else is about to send a fix for the same issue but
> with a different approach of fixing it. i'd like to wait for this 2nd
> patch to arrive before we decide which to merge
Are you sure it wasn't me? :)
You said yesterday that I should send you a patch for it.

The only other approach I could think of would involve a
instance_stop() that waits for the service instance to exit.
I thought initially to do it like this, but decided that waiting
asynchronously for stop event would be a fer better technical solution
to the given issue, don't you agree?

>
> John
>

Alin
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] procd: service instance restart does not wait for old process to be closed before lanching the new one

2017-02-08 Thread Alin Năstac

Hi John,

One of the daemons I use takes sometime a couple of seconds to close
after receiving SIGTERM, so when I issue "/etc/init.d/mydaemon
restart" there will be 2 instances of that service running in parallel
until the initial instance will finally manage to handle the SIGTERM
signal. This daemon registers its own ubus object, but new instance
will fail to register its object due to name conflict with an object
that is already existing.

The root cause of this defect is the way procd restarts a service by
issuing a service_stop followed immediately by a service_start:
  1) service_stop call translates to
- instance_stop() ->  send SIGTERM signal to the old instance
- instance_free() -> call uloop_process_delete(), which means
in->proc.pending will be set to 0
  2) sevice_start will launch the new instance becaise previous
process has been already deleted

Shouldn't uloop_process_delete() be called only when process was
closed, as it is the case in uloop.c? If so, instance_start() should
at least set restart to 1 when (in->proc.pending && in->halt).

And isn't a bit too optimistic to assume all daemons will be stopped by SIGTERM?

BR,
Alin
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

[OpenWrt-Devel] procd: service instance restart does not wait for old process to be closed before lanching the new one

2017-02-08 Thread Alin Năstac

Hi John,

One of the daemons I use takes sometime a couple of seconds to close
after receiving SIGTERM, so when I issue "/etc/init.d/mydaemon
restart" there will be 2 instances of that service running in parallel
until the initial instance will finally manage to handle the SIGTERM
signal. This daemon registers its own ubus object, but new instance
will fail to register its object due to name conflict with an object
that is already existing.

The root cause of this defect is the way procd restarts a service by
issuing a service_stop followed immediately by a service_start:
  1) service_stop call translates to
- instance_stop() ->  send SIGTERM signal to the old instance
- instance_free() -> call uloop_process_delete(), which means
in->proc.pending will be set to 0
  2) sevice_start will launch the new instance becaise previous
process has been already deleted

Shouldn't uloop_process_delete() be called only when process was
closed, as it is the case in uloop.c? If so, instance_start() should
at least set restart to 1 when (in->proc.pending && in->halt).

And isn't a bit too optimistic to assume all daemons will be stopped by SIGTERM?

BR,
Alin
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] ubus/libubox: SIGTERM/SIGINT signals received during ubus_complete_request() calls are ignored

2017-02-03 Thread Alin Năstac

On Fri, Feb 3, 2017 at 4:19 PM, Felix Fietkau <n...@nbd.name> wrote:
> On 2017-02-03 15:57, Alin Năstac wrote:
>> Hi Felix,
>>
>> The SIGTERM ignore issue I was experiencing before is no longer
>> reproducible after I apply your patch.
>>
>> However, I'm concerned about a possible ignore of SIGTERM signal
>> received during a ubus_complete_request() call. If ctx->stack_depth is
>> 0, any such signal received between prev_uloop_initialization and the
>> reset of ulopp_cancelling to false will be ignored. Is this
>> "uloop_cancelling = false" really necessary?
>>
>> BTW, I think the reset of uloop_status and uloop_cancelled should be
>> executed before uloop_setup_signals() like so:
>> if (!recursive_calls++) {
>>uloop_status = 0;
>>uloop_cancelled = false;
>>uloop_setup_signals(true);
>> }
> I was worried about the corner case of performing a ubus request after
> uloop_run has already completed.
> I guess one way this could be addressed is by setting uloop_cancelled =
> false at the end of uloop_run().

How about adding this uloop function:
static int recursive_calls = 0; /* moved from uloop_run() context */
int uloop_cancelling()
{
return recursive_calls > 0 && uloop_cancelled;
}

This function will return true only when uloop_run() is still running,
so you will no longer need to touch uloop_cancelled state at all. This
way you could reduce the scope of uloop_cancelled to uloop.c too.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] ubus/libubox: SIGTERM/SIGINT signals received during ubus_complete_request() calls are ignored

2017-02-03 Thread Alin Năstac

Hi Felix,

The SIGTERM ignore issue I was experiencing before is no longer
reproducible after I apply your patch.

However, I'm concerned about a possible ignore of SIGTERM signal
received during a ubus_complete_request() call. If ctx->stack_depth is
0, any such signal received between prev_uloop_initialization and the
reset of ulopp_cancelling to false will be ignored. Is this
"uloop_cancelling = false" really necessary?

BTW, I think the reset of uloop_status and uloop_cancelled should be
executed before uloop_setup_signals() like so:
if (!recursive_calls++) {
   uloop_status = 0;
   uloop_cancelled = false;
   uloop_setup_signals(true);
}

Cheers,
Alin

On Fri, Feb 3, 2017 at 2:47 PM, Felix Fietkau <n...@nbd.name> wrote:
> Hi Alin,
>
> On 2017-02-03 09:29, Alin Năstac wrote:
>> Hi Felix,
>>
>> SIGTERM & SIGINT signals received during ubus_complete_request()
>> waiting for ubus_poll_data() to return are ignored due to
>> uloop_cancelled being restored to its previous value it had before
>> uloop_poll_data() was called.
>>
>> The reproduction scenario is this:
>>   1) cancelled local variable is set to false, current value of 
>> uloop_cancelled
>>   2) while ubus_poll_data() is waiting for a read event, a SIGTERM is
>> received, so uloop_cancelled is set to true
>>   3) after ubus_poll_data() returns, uloop_cancelled value gets
>> overwritten with false and SIGTERM signal ends up being ignored in the
>> uloop_run()
>>
>> The whole uloop_cancelled related logic in the ubus_complete_request()
>> seems to be focused on getting out from the current recursion of
>> uloop_run(), but from what I see uloop_run() capability to be called
>> recursively is no longer used in ubus, so I wonder if it is still
>> necessary.
> I left in uloop_cancelled primarily to deal with cleaning up recursive
> requests after Ctrl+c or SIGTERM when uloop is in use.
>
>> If the answer is no, the entire logic referring to uloop_cancelled and
>> uloop_end() should be removed from libubus-req.c. Otherwise an
>> additional uloop_cancelled_recursions bit mask would be needed that
>> will allow to control uloop_run() exit condition for each individual
>> recursion.
> I think the main problem was the fact that uloop_cancelled was used
> both for internal request termination and also for external use.
> Here's a patch that should resolve this issue, please test:
>
> ---
> diff --git a/libubus-io.c b/libubus-io.c
> index 1075c65..1343bb2 100644
> --- a/libubus-io.c
> +++ b/libubus-io.c
> @@ -154,9 +154,10 @@ int __hidden ubus_send_msg(struct ubus_context *ctx, 
> uint32_t seq,
> return ret;
>  }
>
> -static int recv_retry(int fd, struct iovec *iov, bool wait, int *recv_fd)
> +static int recv_retry(struct ubus_context *ctx, struct iovec *iov, bool 
> wait, int *recv_fd)
>  {
> int bytes, total = 0;
> +   int fd = ctx->sock.fd;
> static struct {
> struct cmsghdr h;
> int fd;
> @@ -191,7 +192,7 @@ static int recv_retry(int fd, struct iovec *iov, bool 
> wait, int *recv_fd)
>
> if (bytes < 0) {
> bytes = 0;
> -   if (uloop_cancelled)
> +   if (uloop_cancelled || ctx->cancel_poll)
> return 0;
> if (errno == EINTR)
> continue;
> @@ -274,7 +275,7 @@ static bool get_next_msg(struct ubus_context *ctx, int 
> *recv_fd)
> int r;
>
> /* receive header + start attribute */
> -   r = recv_retry(ctx->sock.fd, , false, recv_fd);
> +   r = recv_retry(ctx, , false, recv_fd);
> if (r <= 0) {
> if (r < 0)
> ctx->sock.eof = true;
> @@ -298,7 +299,7 @@ static bool get_next_msg(struct ubus_context *ctx, int 
> *recv_fd)
> iov.iov_base = (char *)ctx->msgbuf.data + sizeof(hdrbuf.data);
> iov.iov_len = blob_len(ctx->msgbuf.data);
> if (iov.iov_len > 0 &&
> -   recv_retry(ctx->sock.fd, , true, NULL) <= 0)
> +   recv_retry(ctx, , true, NULL) <= 0)
> return false;
>
> return true;
> @@ -311,7 +312,7 @@ void __hidden ubus_handle_data(struct uloop_fd *u, 
> unsigned int events)
>
> while (get_next_msg(ctx, _fd)) {
> ubus_process_msg(ctx, >msgbuf, recv_fd);
> -   if (uloop_cancelled)
> +   if (uloop_cancelled || ctx->cancel_poll)
> break;
> }
>
&g

[OpenWrt-Devel] ubus/libubox: SIGTERM/SIGINT signals received during ubus_complete_request() calls are ignored

2017-02-03 Thread Alin Năstac

Hi Felix,

SIGTERM & SIGINT signals received during ubus_complete_request()
waiting for ubus_poll_data() to return are ignored due to
uloop_cancelled being restored to its previous value it had before
uloop_poll_data() was called.

The reproduction scenario is this:
  1) cancelled local variable is set to false, current value of uloop_cancelled
  2) while ubus_poll_data() is waiting for a read event, a SIGTERM is
received, so uloop_cancelled is set to true
  3) after ubus_poll_data() returns, uloop_cancelled value gets
overwritten with false and SIGTERM signal ends up being ignored in the
uloop_run()

The whole uloop_cancelled related logic in the ubus_complete_request()
seems to be focused on getting out from the current recursion of
uloop_run(), but from what I see uloop_run() capability to be called
recursively is no longer used in ubus, so I wonder if it is still
necessary.

If the answer is no, the entire logic referring to uloop_cancelled and
uloop_end() should be removed from libubus-req.c. Otherwise an
additional uloop_cancelled_recursions bit mask would be needed that
will allow to control uloop_run() exit condition for each individual
recursion.

Please share your pov so I could prepare the necessary patches.

Thanks,
Alin
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [LEDE-DEV] [PATCH] conntrack: enable support for netfilter conntrack zones

2016-05-20 Thread Alin Năstac

Hi Jo,

You have my ACK. ;)
Sorry about that, I will sign my patches from now on.

BR,
Alin

On Thu, May 19, 2016 at 6:21 PM, Jo-Philipp Wich  wrote:
> Hi Alin,
>
> I merged your patch into my staging tree at
>
> https://git.lede-project.org/?p=lede/jow/staging.git;a=commitdiff;h=6c9231baa9c5341c6ee2e213618dcde72d42288b
>
> Since your change lacked a proper Signed-off-by I added it on your
> behalf. Please review the link above and give me your ACK, then I'll
> push it to master after some compile testing.
>
> Regards,
> Jo
>
> On 05/19/2016 09:54 AM, Alin Nastac wrote:
>> Storage of such zones is provided by a nf_ct_ext struct, hence conntrack
>> memory foot print will not be increased if zones are not used.
>> ---
>>  package/kernel/linux/modules/netfilter.mk | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/package/kernel/linux/modules/netfilter.mk 
>> b/package/kernel/linux/modules/netfilter.mk
>> index 3b623e4..4d9c116 100644
>> --- a/package/kernel/linux/modules/netfilter.mk
>> +++ b/package/kernel/linux/modules/netfilter.mk
>> @@ -68,6 +68,7 @@ define KernelPackage/nf-conntrack
>>KCONFIG:= \
>>  CONFIG_NETFILTER=y \
>>  CONFIG_NETFILTER_ADVANCED=y \
>> +CONFIG_NF_CONNTRACK_ZONES=y \
>>   $(KCONFIG_NF_CONNTRACK)
>>FILES:=$(foreach mod,$(NF_CONNTRACK-m),$(LINUX_DIR)/net/$(mod).ko)
>>AUTOLOAD:=$(call AutoProbe,$(notdir $(NF_CONNTRACK-m)))
>>
>
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH firewall] zones : Redirect incoming WAN traffic only when the destination IP address matches the IP masquerading address

2015-10-05 Thread Alin Năstac

Here is the original description I gave to my patch (see
http://patchwork.ozlabs.org/patch/516167/):

Basically it prevents zone_wan_prerouting rules to affect traffic towards
IP addresses that are not used
for masquerading LAN private IP space and it does that by setting
destination IP address of the
delegate_prerouting rules for zone with masq enabled to whatever
address(es) that particular network
interface has.

The typical scenario this patch fixes involves 2 LAN network prefixes:
  - the usual 192.168.1.0/24 which is masqueraded by the public IP address
configured on the WAN interface
  - a public IP network prefix for those LAN devices that are supposed to
be excluded from NAT
Without this patch, port forwarding rules introduced for 192.168.1.x LAN
devices will also affect traffic
towards the 2nd prefix.


On Thu, Oct 1, 2015 at 10:05 PM, Jo-Philipp Wich  wrote:

> Hi,
>
> wouldn't this break port forwards to hosts not being within the range of
> the on-link lan subnet?
>
> I also read the patch description three times and still am not sure what
> that change attempts to achive.
>
> Can you further explain the problem please and provide a before/after
> "fw3 print" diff so that I better understand your proposed solution?
>
> ~ Jow
>
>
> Am 01.10.2015 um 18:38 schrieb Hans Dedecker:
> > This patch fixes an issue when 2 LAN network prefixes are in use :
> >  - the usual 192.168.0.0/24 which is masqueraded by the public IP
> address on the
> >WAN interface
> >  - a public IP network prefix for those LAN devices that are excluded
> from NAT
> >
> > Port forwarding rules introduced for 192.168.1.x devices will currently
> also
> > translate traffic addressed to the public network addresses in use on
> the LAN
> > as the destination address in the delegate prerouting rule(s) is unset.
> > The patch sets the destination IP address(es) in the delegate prerouting
> rules
> > equal to the IP address(es) that particular network interface has as
> extra descriminator
> >
> > Signed-off-by: Hans Dedecker 
> > Signed-off-by: Alin Nastac 
> > ---
> >  zones.c | 36 
> >  1 file changed, 32 insertions(+), 4 deletions(-)
> >
> > diff --git a/zones.c b/zones.c
> > index 2ddd7b4..8bd6673 100644
> > --- a/zones.c
> > +++ b/zones.c
> > @@ -383,10 +383,38 @@ print_interface_rule(struct fw3_ipt_handle
> *handle, struct fw3_state *state,
> >   {
> >   if (has(zone->flags, handle->family, FW3_FLAG_DNAT))
> >   {
> > - r = fw3_ipt_rule_create(handle, NULL, dev, NULL,
> sub, NULL);
> > - fw3_ipt_rule_target(r, "zone_%s_prerouting",
> zone->name);
> > - fw3_ipt_rule_extra(r, zone->extra_src);
> > - fw3_ipt_rule_replace(r, "delegate_prerouting");
> > + struct list_head *addrs;
> > + struct fw3_address *addr;
> > +
> > + addrs = zone->masq ? calloc(1, sizeof(*addrs)) :
> NULL;
> > + if (addrs)
> > + {
> > + /* redirect only the traffic towards a
> locally configured address */
> > + INIT_LIST_HEAD(addrs);
> > + fw3_ubus_address(addrs, dev->network);
> > +
> > + list_for_each_entry(addr, addrs, list)
> > + {
> > + if (!fw3_is_family(addr,
> handle->family))
> > + continue;
> > + /* reset mask to its maximum value
> */
> > + memset(>mask.v6, 0xFF,
> sizeof(addr->mask.v6));
> > +
> > + r = fw3_ipt_rule_create(handle,
> NULL, dev, NULL, sub, addr);
> > + fw3_ipt_rule_target(r,
> "zone_%s_prerouting", zone->name);
> > + fw3_ipt_rule_extra(r,
> zone->extra_src);
> > + fw3_ipt_rule_replace(r,
> "delegate_prerouting");
> > + }
> > +
> > + fw3_free_list(addrs);
> > + }
> > + else
> > + {
> > + r = fw3_ipt_rule_create(handle, NULL, dev,
> NULL, sub, NULL);
> > + fw3_ipt_rule_target(r,
> "zone_%s_prerouting", zone->name);
> > + fw3_ipt_rule_extra(r, zone->extra_src);
> > + fw3_ipt_rule_replace(r,
> "delegate_prerouting");
> > + }
> >   }
> >
> >   if (has(zone->flags, handle->family, FW3_FLAG_SNAT))
> >
>
>
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org

[OpenWrt-Devel] [PATCH] [package] firewall: Redirect incoming WAN traffic only when destination IP address matches the IP address used for masquerading

2015-09-09 Thread Alin Năstac

This is a git patch for the firewall3 git repo at git://nbd.name/firewall3.git

Basically it prevents zone_wan_prerouting rules to affect traffic
towards IP addresses that are not used for masquerading LAN private IP
space and it does that by setting destination IP address of the
delegate_prerouting rules for zone with masq enabled to whatever
address(es) that particular network interface has.

The typical scenario this patch fixes involves 2 LAN network prefixes:
  - the usual 192.168.1.0/24 which is masqueraded by the public IP
address configured on the WAN interface
  - a public IP network prefix for those LAN devices that are supposed
to be excluded from NAT
Without this patch, port forwarding rules introduced for 192.168.1.x
LAN devices will also affect traffic towards the 2nd prefix.

>From 56820e2e3e09f68e4f9a74e6aff832fbcf2c5729 Mon Sep 17 00:00:00 2001
From: Alin Nastac 
Date: Fri, 4 Sep 2015 13:54:10 +0200
Subject: [PATCH] Redirect incoming WAN traffic only when
 destination IP address matches the IP address configured on the
incoming interface

---
 zones.c | 36 
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/zones.c b/zones.c
index 2ddd7b4..8bd6673 100644
--- a/zones.c
+++ b/zones.c
@@ -383,10 +383,38 @@ print_interface_rule(struct fw3_ipt_handle
*handle, struct fw3_state *state,
{
if (has(zone->flags, handle->family, FW3_FLAG_DNAT))
{
-   r = fw3_ipt_rule_create(handle, NULL, dev,
NULL, sub, NULL);
-   fw3_ipt_rule_target(r, "zone_%s_prerouting",
zone->name);
-   fw3_ipt_rule_extra(r, zone->extra_src);
-   fw3_ipt_rule_replace(r, "delegate_prerouting");
+   struct list_head *addrs;
+   struct fw3_address *addr;
+
+   addrs = zone->masq ? calloc(1, sizeof(*addrs)) : NULL;
+   if (addrs)
+   {
+   /* redirect only the traffic towards a
locally configured address */
+   INIT_LIST_HEAD(addrs);
+   fw3_ubus_address(addrs, dev->network);
+
+   list_for_each_entry(addr, addrs, list)
+   {
+   if (!fw3_is_family(addr,
handle->family))
+   continue;
+   /* reset mask to its maximum value */
+   memset(>mask.v6, 0xFF,
sizeof(addr->mask.v6));
+
+   r =
fw3_ipt_rule_create(handle, NULL, dev, NULL, sub, addr);
+   fw3_ipt_rule_target(r,
"zone_%s_prerouting", zone->name);
+   fw3_ipt_rule_extra(r, zone->extra_src);
+   fw3_ipt_rule_replace(r,
"delegate_prerouting");
+   }
+
+   fw3_free_list(addrs);
+   }
+   else
+   {
+   r = fw3_ipt_rule_create(handle, NULL,
dev, NULL, sub, NULL);
+   fw3_ipt_rule_target(r,
"zone_%s_prerouting", zone->name);
+   fw3_ipt_rule_extra(r, zone->extra_src);
+   fw3_ipt_rule_replace(r, "delegate_prerouting");
+   }
}

if (has(zone->flags, handle->family, FW3_FLAG_SNAT))
--
1.7.12.4
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [PATCH odhcpd 2/2] router: always check ra_lifetime

Re: [PATCH][netifd] system-linux: initialize ifreq struct before using it

Re: [PATCH][netifd] system-linux: initialize ifreq struct before using it

Re: [OpenWrt-Devel] [PATCH] iproute2: revert add libcap support, enabled in ip-full

Re: [OpenWrt-Devel] [PATCH] iproute2: revert add libcap support, enabled in ip-full

Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix route ownership to kernel when IPv6 address becomes deprecated

Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix route ownership to kernel when IPv6 address becomes deprecated

Re: [OpenWrt-Devel] [netifd][PATCH] interface-ip: transfer prefix route ownership to kernel when IPv6 address becomes deprecated

Re: [OpenWrt-Devel] [PATCH] interface: add IPv6 addresses without IFA_F_NOPREFIXROUTE

Re: [OpenWrt-Devel] [PATCH v2] fstools: add a hook before mounting the overlay

Re: [OpenWrt-Devel] [PATCH] fstools: add a hook before mounting the overlay

Re: [OpenWrt-Devel] [PATCH] fstools: add a hook before mounting the overlay

Re: [OpenWrt-Devel] [PATCH] firewall3: make reject types selectable by user

Re: [OpenWrt-Devel] [PATCH] firewall3: make reject types selectable by user

Re: [OpenWrt-Devel] [PATCH] procd: service gets deleted when its last instance is freed

Re: [OpenWrt-Devel] [LEDE-DEV] [PATCH] procd: stop service using SIGKILL if SIGTERM failed to do so

Re: [OpenWrt-Devel] procd: service instance restart does not wait for old process to be closed before lanching the new one

[OpenWrt-Devel] procd: service instance restart does not wait for old process to be closed before lanching the new one

Re: [OpenWrt-Devel] ubus/libubox: SIGTERM/SIGINT signals received during ubus_complete_request() calls are ignored

Re: [OpenWrt-Devel] ubus/libubox: SIGTERM/SIGINT signals received during ubus_complete_request() calls are ignored

[OpenWrt-Devel] ubus/libubox: SIGTERM/SIGINT signals received during ubus_complete_request() calls are ignored

Re: [OpenWrt-Devel] [LEDE-DEV] [PATCH] conntrack: enable support for netfilter conntrack zones

Re: [OpenWrt-Devel] [PATCH firewall] zones : Redirect incoming WAN traffic only when the destination IP address matches the IP masquerading address

[OpenWrt-Devel] [PATCH] [package] firewall: Redirect incoming WAN traffic only when destination IP address matches the IP address used for masquerading

24 matches

Site Navigation

Mail list logo

Footer information