Re: [PATCH] eth-multiplxer: Implement ds_device_close()

2017-09-22 Thread Joan Lledó 
> Please confirm that eth-multiplexer does not die.

It doesn't, see [1]

> MIG_SERVER_DIED is generated by the mig-generated client stubs when the
> replies message id does not matche the expected id, but
> MACH_NOTIFY_SEND_ONCE.  This message is generated by the kernel if a
> send-once right is destroyed.  This usually happens when the server
> dies, but could in theory happen when the server explicitly destroys the
> reply port, but tbh I couldn't see why or where that should happen.

I checked with gdb that the client returns in [2], line 173

-
[1] http://paste.debian.net/987211/
[2] http://paste.debian.net/987228/

Re: [PATCH] eth-multiplxer: Implement ds_device_close()

2017-09-21 Thread Justus Winter 
Hi :)

"Joan Lledó"  writes:

> Hello,
>
> One difference between pfinet and lwip translators is that pfinet doesn't 
> delete interfaces (or I haven't found how to do it) while lwip does. For 
> instance, if one starts pfinet with:
>
> settrans -fga /servers/socket/2 /hurd/pfinet -i /dev/eth1 -a 192.168.123.178 
> -m 255.255.255.0 -g 192.168.123.1
>
> and then runs:
>
> fsysopts /servers/socket/2 --interface=/dev/eth2 --address=192.168.124.178 
> --netmask=255.255.255.0 --gateway=192.168.124.1
>
> the result is this:
>
> root@hurd:/home/jlledom# fsysopts /servers/socket/2
> /hurd/pfinet --interface=/dev/eth1 --address=192.168.123.178 
> --netmask=255.255.255.0 --address6=fc00:123::5054:ff:feb6:1ab3/64 
> --address6=fe80::5254:b6:1ab3/10 --address6=fe80::5054:ff:feb6:1ab3/10 
> --gateway6=fe80::5054:ff:fef6:d496 --interface=/dev/eth2 
> --address=192.168.124.178 --netmask=255.255.255.0 --gateway=192.168.124.1 
> --address6=fe80::5254:4b:ad11/10 --address6=fe80::5054:ff:fe4b:ad11/10
>
> fsysopts added a new interface. But doing the same with lwip leads to this:
>
> root@hurd:/home/jlledom# fsysopts /servers/socket/2
> /hurd/lwip --interface=/dev/eth2 --address=192.168.124.178 
> --netmask=255.255.255.0 --gateway=192.168.124.1 
> --address6=FE80::5054:FF:FE4B:AD11/64 
> --address6=FC00:124::5054:FF:FE4B:AD11/64
>
> The stack is reset and reconfigured, and only the given interface is added. I 
> implemented this by simply removing all interfaces anytime somebody calls 
> fsysopts, and adding the new ones. If some interface already exists, it's 
> deleted and created again.

Afaik using multiple interfaces is not something that many people have
done, so either behavior looks okay to me.

> This behaviour generated a problem when working with
> eth-multiplexer. Adding a new interface calls ds_device_open in
> eth-multiplexer and that returns a port name, whereas deleting the
> interface deallocates the port name in lwip, but nothing happens in
> eth-multiplexer since ds_device_close is unimplemented,

ds_device_close is not unimplemented, it merely does nothing.  But
indeed, nothing happens when the only sender deallocates its port
because of the way eth-multiplexer handles the device objects (it puts
them into a linked list with a hard reference, preventing the proper
deallocation of the port).  I'll try to fix that.

> when lwip
> tries to create the same interface again, eth-multiplexer returns the
> same port name,

That should be fine though.

> and trying to use it leads to a MIG_SERVER_DIED
> error.

Please confirm that eth-multiplexer does not die.

MIG_SERVER_DIED is generated by the mig-generated client stubs when the
replies message id does not matche the expected id, but
MACH_NOTIFY_SEND_ONCE.  This message is generated by the kernel if a
send-once right is destroyed.  This usually happens when the server
dies, but could in theory happen when the server explicitly destroys the
reply port, but tbh I couldn't see why or where that should happen.


Cheers,
Justus

Re: [PATCH] eth-multiplxer: Implement ds_device_close()

2017-09-20 Thread Joan Lledó 
Ooops!! I forgot that! :)

2017-09-20 20:06 GMT+02:00 James Clarke :
> On 20 Sep 2017, at 11:08, Joan Lledó  wrote:
>>
>> ---
>> eth-multiplexer/Makefile  | 2 +-
>> eth-multiplexer/device_impl.c | 5 +
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/eth-multiplexer/Makefile b/eth-multiplexer/Makefile
>> index cefa0abd..2b19de6d 100644
>> --- a/eth-multiplexer/Makefile
>> +++ b/eth-multiplexer/Makefile
>> @@ -29,6 +29,6 @@ LCLHDRS = ethernet.h util.h vdev.h netfs_impl.h
>> HURDLIBS = ports ihash iohelp fshelp shouldbeinlibc netfs bpf
>> LDLIBS = -lpthread
>>
>> -CFLAGS += -I$(top_srcdir)/libbpf
>> +CFLAGS += -I$(top_srcdir)/libbpf -ggdb -O0
>
> I don't think you meant to include this hunk? :)
>
> James
>

Re: [PATCH] eth-multiplxer: Implement ds_device_close()

2017-09-20 Thread James Clarke 
On 20 Sep 2017, at 11:08, Joan Lledó  wrote:
> 
> ---
> eth-multiplexer/Makefile  | 2 +-
> eth-multiplexer/device_impl.c | 5 +
> 2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/eth-multiplexer/Makefile b/eth-multiplexer/Makefile
> index cefa0abd..2b19de6d 100644
> --- a/eth-multiplexer/Makefile
> +++ b/eth-multiplexer/Makefile
> @@ -29,6 +29,6 @@ LCLHDRS = ethernet.h util.h vdev.h netfs_impl.h
> HURDLIBS = ports ihash iohelp fshelp shouldbeinlibc netfs bpf
> LDLIBS = -lpthread
> 
> -CFLAGS += -I$(top_srcdir)/libbpf
> +CFLAGS += -I$(top_srcdir)/libbpf -ggdb -O0

I don't think you meant to include this hunk? :)

James

[PATCH] eth-multiplxer: Implement ds_device_close()

2017-09-20 Thread Joan Lledó 
---
 eth-multiplexer/Makefile  | 2 +-
 eth-multiplexer/device_impl.c | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/eth-multiplexer/Makefile b/eth-multiplexer/Makefile
index cefa0abd..2b19de6d 100644
--- a/eth-multiplexer/Makefile
+++ b/eth-multiplexer/Makefile
@@ -29,6 +29,6 @@ LCLHDRS = ethernet.h util.h vdev.h netfs_impl.h
 HURDLIBS = ports ihash iohelp fshelp shouldbeinlibc netfs bpf
 LDLIBS = -lpthread
 
-CFLAGS += -I$(top_srcdir)/libbpf
+CFLAGS += -I$(top_srcdir)/libbpf -ggdb -O0
 
 include ../Makeconf
diff --git a/eth-multiplexer/device_impl.c b/eth-multiplexer/device_impl.c
index d7c8beeb..b84a3309 100644
--- a/eth-multiplexer/device_impl.c
+++ b/eth-multiplexer/device_impl.c
@@ -96,6 +96,11 @@ ds_device_open (mach_port_t master_port, mach_port_t 
reply_port,
 kern_return_t
 ds_device_close (struct vether_device *device)
 {
+  if (device == NULL)
+return D_NO_SUCH_DEVICE;
+
+  destroy_vdev(device);
+
   return 0;
 }
 
-- 
2.11.0

[PATCH] eth-multiplxer: Implement ds_device_close()

2017-09-20 Thread Joan Lledó 
Hello,

One difference between pfinet and lwip translators is that pfinet doesn't 
delete interfaces (or I haven't found how to do it) while lwip does. For 
instance, if one starts pfinet with:

settrans -fga /servers/socket/2 /hurd/pfinet -i /dev/eth1 -a 192.168.123.178 -m 
255.255.255.0 -g 192.168.123.1

and then runs:

fsysopts /servers/socket/2 --interface=/dev/eth2 --address=192.168.124.178 
--netmask=255.255.255.0 --gateway=192.168.124.1

the result is this:

root@hurd:/home/jlledom# fsysopts /servers/socket/2
/hurd/pfinet --interface=/dev/eth1 --address=192.168.123.178 
--netmask=255.255.255.0 --address6=fc00:123::5054:ff:feb6:1ab3/64 
--address6=fe80::5254:b6:1ab3/10 --address6=fe80::5054:ff:feb6:1ab3/10 
--gateway6=fe80::5054:ff:fef6:d496 --interface=/dev/eth2 
--address=192.168.124.178 --netmask=255.255.255.0 --gateway=192.168.124.1 
--address6=fe80::5254:4b:ad11/10 --address6=fe80::5054:ff:fe4b:ad11/10

fsysopts added a new interface. But doing the same with lwip leads to this:

root@hurd:/home/jlledom# fsysopts /servers/socket/2
/hurd/lwip --interface=/dev/eth2 --address=192.168.124.178 
--netmask=255.255.255.0 --gateway=192.168.124.1 
--address6=FE80::5054:FF:FE4B:AD11/64 --address6=FC00:124::5054:FF:FE4B:AD11/64

The stack is reset and reconfigured, and only the given interface is added. I 
implemented this by simply removing all interfaces anytime somebody calls 
fsysopts, and adding the new ones. If some interface already exists, it's 
deleted and created again.

This behaviour generated a problem when working with eth-multiplexer. Adding a 
new interface calls ds_device_open in eth-multiplexer and that returns a port 
name, whereas deleting the interface deallocates the port name in lwip, but 
nothing happens in eth-multiplexer since ds_device_close is unimplemented, when 
lwip tries to create the same interface again, eth-multiplexer returns the same 
port name, and trying to use it leads to a MIG_SERVER_DIED error. This use to 
happen when ifupdown uses dhclient to get an address and is not a problem in 
pfinet since it doesn't remove the interface.

I wrote a little patch to implement ds_device_close(). This patch is working 
for me, but I'm not sure whether it's the right way to do it or may affect 
other servers. I'd appreciate some feedback.

Regards,

Joan