Alexander Graf wrote:
> 
> On 28.05.2009, at 23:31, Jan Kiszka wrote:
> 
>> Alexander Graf wrote:
>>> Using the new host_net_redir command you can easily create redirections
>>> on the fly while your VM is running.
>>>
>>> While that's great, it's missing the removal of redirections, in case
>>> you
>>> want to have a port closed again at a later point in time.
>>>
>>> This patch adds support for removal of redirections.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> monitor.c        |    5 +++--
>>> net.c            |   43 ++++++++++++++++++++++++++++++++++++++++++-
>>> net.h            |    2 +-
>>> slirp/libslirp.h |    1 +
>>> slirp/slirp.c    |   23 +++++++++++++++++++++++
>>> vl.c             |    2 +-
>>> 6 files changed, 71 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index 0f38c71..dbab3de 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -1759,8 +1759,9 @@ static const mon_cmd_t mon_cmds[] = {
>>>     { "host_net_remove", "is", net_host_device_remove,
>>>       "vlan_id name", "remove host VLAN client" },
>>> #ifdef CONFIG_SLIRP
>>> -    { "host_net_redir", "s", net_slirp_redir,
>>> -      "[tcp|udp]:host-port:[guest-host]:guest-port", "redirect TCP
>>> or UDP connections from host to guest (requires -net user)" },
>>> +    { "host_net_redir", "ss?", net_slirp_redir,
>>> +      "[tcp|udp]:host-port:[guest-host]:guest-port", "redirect TCP
>>> or UDP connections from host to guest (requires -net user)\n"
>>> +      "host_net_redir remove [tcp:|udp:]host-port -- remove
>>> redirection" },
>>> #endif
>>>     { "balloon", "i", do_balloon,
>>>       "target", "request VM to change it's memory allocation (in MB)" },
>>> diff --git a/net.c b/net.c
>>> index 31ee95a..de019b3 100644
>>> --- a/net.c
>>> +++ b/net.c
>>> @@ -568,7 +568,43 @@ static int net_slirp_init(VLANState *vlan, const
>>> char *model, const char *name)
>>>     return 0;
>>> }
>>>
>>> -void net_slirp_redir(Monitor *mon, const char *redir_str)
>>> +static void net_slirp_redir_rm(Monitor *mon, const char *port_str)
>>> +{
>>> +    int host_port;
>>> +    char buf[256] = "";
>>> +    const char *p = port_str;
>>> +    int is_udp = 0;
>>> +    int n;
>>> +
>>> +    if (!mon)
>>> +        return;
>>> +
>>> +    if (!port_str || !port_str[0])
>>> +        goto fail_syntax;
>>> +
>>> +    get_str_sep(buf, sizeof(buf), &p, ':');
>>> +
>>> +    if (!strcmp(buf, "tcp") || buf[0] == '\0') {
>>> +        is_udp = 0;
>>> +    } else if (!strcmp(buf, "udp")) {
>>> +        is_udp = 1;
>>> +    } else {
>>> +        goto fail_syntax;
>>> +    }
>>> +
>>> +    host_port = atoi(p);
>>> +
>>> +    n = slirp_redir_rm(is_udp, host_port);
>>> +
>>> +    monitor_printf(mon, "removed %d redirections to %s port %d\n", n,
>>> +                        is_udp ? "udp" : "tcp", host_port);
>>> +    return;
>>> +
>>> + fail_syntax:
>>> +    monitor_printf(mon, "invalid format\n");
>>> +}
>>> +
>>> +void net_slirp_redir(Monitor *mon, const char *redir_str, const char
>>> *redir_opt2)
>>> {
>>>     int is_udp;
>>>     char buf[256], *r;
>>> @@ -581,6 +617,11 @@ void net_slirp_redir(Monitor *mon, const char
>>> *redir_str)
>>>         slirp_init(slirp_restrict, slirp_ip);
>>>     }
>>>
>>> +    if (!strcmp(redir_str, "remove")) {
>>> +        net_slirp_redir_rm(mon, redir_opt2);
>>> +        return;
>>> +    }
>>> +
>>>     p = redir_str;
>>>     if (get_str_sep(buf, sizeof(buf), &p, ':') < 0)
>>>         goto fail_syntax;
>>> diff --git a/net.h b/net.h
>>> index 41a3082..feee021 100644
>>> --- a/net.h
>>> +++ b/net.h
>>> @@ -112,7 +112,7 @@ int net_client_init(const char *device, const
>>> char *p);
>>> void net_client_uninit(NICInfo *nd);
>>> int net_client_parse(const char *str);
>>> void net_slirp_smb(const char *exported_dir);
>>> -void net_slirp_redir(Monitor *mon, const char *redir_str);
>>> +void net_slirp_redir(Monitor *mon, const char *redir_str, const char
>>> *redir_opt2);
>>> void net_cleanup(void);
>>> int slirp_is_inited(void);
>>> void net_client_check(void);
>>> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
>>> index a1cd70e..6fc2c32 100644
>>> --- a/slirp/libslirp.h
>>> +++ b/slirp/libslirp.h
>>> @@ -18,6 +18,7 @@ void slirp_input(const uint8_t *pkt, int pkt_len);
>>> int slirp_can_output(void);
>>> void slirp_output(const uint8_t *pkt, int pkt_len);
>>>
>>> +int slirp_redir_rm(int is_udp, int host_port);
>>> int slirp_redir(int is_udp, int host_port,
>>>                 struct in_addr guest_addr, int guest_port);
>>> int slirp_add_exec(int do_pty, const void *args, int addr_low_byte,
>>> diff --git a/slirp/slirp.c b/slirp/slirp.c
>>> index 04d3ded..33397c0 100644
>>> --- a/slirp/slirp.c
>>> +++ b/slirp/slirp.c
>>> @@ -734,6 +734,29 @@ void if_encap(const uint8_t *ip_data, int
>>> ip_data_len)
>>>     }
>>> }
>>>
>>> +/* Unlistens a redirection
>>> + *
>>> + * Return value: number of redirs removed */
>>> +int slirp_redir_rm(int is_udp, int host_port)
>>> +{
>>> +    struct socket *so;
>>> +    struct socket *head = (is_udp ? &udb : &tcb);
>>> +    int fport = htons(host_port);
>>> +    int n = 0;
>>> +
>>> + loop_again:
>>> +    for (so = head->so_next; so != head; so = so->so_next) {
>>> +        if (so->so_fport == fport) {
>>> +            close(so->s);
>>> +            sofree(so);
>>> +            n++;
>>> +            goto loop_again;
>>> +        }
>>> +    }
>>
>> Unfortunately, this does not only target host->guest redirection sockets
>> but also sockets slirp uses for NAT'ing guest originated connections.
>> The same applies to your "host_net_redir list". So giving this in user
>> hand, unwanted damaged can be caused to guest network sessions. What we
>> need is a tag in struct socket to identify static redirection sockets.
> 
> Right - with the current infrastructure there's no easy way to find out
> if a socket was created for incoming (listen) or outgoing purposes FWIW.
> So a flag might make sense.
> 
> I'm not really sure if that works out for UDP sockets though.

Why not? We will tag the socket during tcp/udp_listen, right after
socreate e.g.

> 
>> What's your current plan regarding these two commits? We should
>> coordinate as my work touches the same area.
> 
> Eh - good question. I'd like to get some means of removing and listing
> redirs upstream. The best case for me would be if you'd take that into
> your trees so I don't have to worry about it :-).

I was afraid you'll say this :). As you patches were already merged, I
have to deal with them earlier than expected anyway. OK, will pick this up.

Jan