[PATCH 1/1] include/qemu: Provide a C++ compatible version of typeof_strip_qual

2024-06-24 Thread Felix Wu
From: Roman Kiryanov 

to use the QEMU headers with a C++ compiler.

Signed-off-by: Felix Wu 
Signed-off-by: Roman Kiryanov 
---
 include/qemu/atomic.h   |  8 
 include/qemu/atomic.hpp | 38 ++
 2 files changed, 46 insertions(+)
 create mode 100644 include/qemu/atomic.hpp

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 99110abefb..aeaecc440a 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -20,6 +20,13 @@
 /* Compiler barrier */
 #define barrier()   ({ asm volatile("" ::: "memory"); (void)0; })
 
+#ifdef __cplusplus
+
+#ifndef typeof_strip_qual
+#error Use the typeof_strip_qual(expr) definition from atomic.hpp on C++ 
builds.
+#endif
+
+#else  /* __cpluplus */
 /* The variable that receives the old value of an atomically-accessed
  * variable must be non-qualified, because atomic builtins return values
  * through a pointer-type argument as in __atomic_load(&var, &old, MODEL).
@@ -61,6 +68,7 @@
 __builtin_types_compatible_p(typeof(expr), const volatile unsigned 
short), \
 (unsigned short)1, 
\
   (expr)+0))
+#endif  /* __cpluplus */
 
 #ifndef __ATOMIC_RELAXED
 #error "Expecting C11 atomic ops"
diff --git a/include/qemu/atomic.hpp b/include/qemu/atomic.hpp
new file mode 100644
index 00..5844e3d427
--- /dev/null
+++ b/include/qemu/atomic.hpp
@@ -0,0 +1,38 @@
+/*
+ * The C++ definition for typeof_strip_qual used in atomic.h.
+ *
+ * Copyright (C) 2024 Google, Inc.
+ *
+ * Author: Roman Kiryanov 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * See docs/devel/atomics.rst for discussion about the guarantees each
+ * atomic primitive is meant to provide.
+ */
+
+#ifndef QEMU_ATOMIC_HPP
+#define QEMU_ATOMIC_HPP
+
+#include 
+
+/* Match the integer promotion behavior of typeof_strip_qual, see atomic.h */
+template  struct typeof_strip_qual_cpp { using result = 
decltype(+T(0)); };
+
+template <> struct typeof_strip_qual_cpp { using result = bool; };
+template <> struct typeof_strip_qual_cpp { using result = signed 
char; };
+template <> struct typeof_strip_qual_cpp { using result = 
unsigned char; };
+template <> struct typeof_strip_qual_cpp { using result = signed 
short; };
+template <> struct typeof_strip_qual_cpp { using result = 
unsigned short; };
+
+#define typeof_strip_qual(expr) \
+typeof_strip_qual_cpp< \
+std::remove_cv< \
+std::remove_reference< \
+decltype(expr) \
+>::type \
+>::type \
+>::result
+
+#endif /* QEMU_ATOMIC_HPP */
-- 
2.45.2.741.gdbec12cfda-goog




[PATCH 1/2] qom: Rename Object::class into Object::klass

2024-06-24 Thread Felix Wu
From: Roman Kiryanov 

'class' is a C++ keyword and it prevents from
using the QEMU headers with a C++ compiler.

Google-Bug-Id: 331190993
Change-Id: I9ab7d2d77edef654a9c7b7cb9cd01795a6ed65a2
Signed-off-by: Felix Wu 
Signed-off-by: Roman Kiryanov 
---
 hw/core/qdev-properties-system.c |  2 +-
 include/exec/memory.h|  2 +-
 include/qom/object.h |  2 +-
 qom/object.c | 90 
 4 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index f13350b4fb..a6781841af 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -431,7 +431,7 @@ static void set_netdev(Object *obj, Visitor *v, const char 
*name,
 }
 
 if (peers[i]->info->check_peer_type) {
-if (!peers[i]->info->check_peer_type(peers[i], obj->class, errp)) {
+if (!peers[i]->info->check_peer_type(peers[i], obj->klass, errp)) {
 goto out;
 }
 }
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2d7c278b9f..e5bd75956e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1808,7 +1808,7 @@ static inline IOMMUMemoryRegion 
*memory_region_get_iommu(MemoryRegion *mr)
 static inline IOMMUMemoryRegionClass *memory_region_get_iommu_class_nocheck(
 IOMMUMemoryRegion *iommu_mr)
 {
-return (IOMMUMemoryRegionClass *) (((Object *)iommu_mr)->class);
+return (IOMMUMemoryRegionClass *) (((Object *)iommu_mr)->klass);
 }
 
 #define memory_region_is_iommu(mr) (memory_region_get_iommu(mr) != NULL)
diff --git a/include/qom/object.h b/include/qom/object.h
index 13d3a655dd..7afdb261a8 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -153,7 +153,7 @@ struct ObjectClass
 struct Object
 {
 /* private: */
-ObjectClass *class;
+ObjectClass *klass;
 ObjectFree *free;
 GHashTable *properties;
 uint32_t ref;
diff --git a/qom/object.c b/qom/object.c
index 157a45c5f8..133cd08763 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -68,7 +68,7 @@ struct TypeImpl
 const char *parent;
 TypeImpl *parent_type;
 
-ObjectClass *class;
+ObjectClass *klass;
 
 int num_interfaces;
 InterfaceImpl interfaces[MAX_INTERFACES];
@@ -304,11 +304,11 @@ static void type_initialize_interface(TypeImpl *ti, 
TypeImpl *interface_type,
 type_initialize(iface_impl);
 g_free((char *)info.name);
 
-new_iface = (InterfaceClass *)iface_impl->class;
-new_iface->concrete_class = ti->class;
+new_iface = (InterfaceClass *)iface_impl->klass;
+new_iface->concrete_class = ti->klass;
 new_iface->interface_type = interface_type;
 
-ti->class->interfaces = g_slist_append(ti->class->interfaces, new_iface);
+ti->klass->interfaces = g_slist_append(ti->klass->interfaces, new_iface);
 }
 
 static void object_property_free(gpointer data)
@@ -329,7 +329,7 @@ static void type_initialize(TypeImpl *ti)
 {
 TypeImpl *parent;
 
-if (ti->class) {
+if (ti->klass) {
 return;
 }
 
@@ -350,7 +350,7 @@ static void type_initialize(TypeImpl *ti)
 assert(!ti->instance_finalize);
 assert(!ti->num_interfaces);
 }
-ti->class = g_malloc0(ti->class_size);
+ti->klass = g_malloc0(ti->class_size);
 
 parent = type_get_parent(ti);
 if (parent) {
@@ -360,10 +360,10 @@ static void type_initialize(TypeImpl *ti)
 
 g_assert(parent->class_size <= ti->class_size);
 g_assert(parent->instance_size <= ti->instance_size);
-memcpy(ti->class, parent->class, parent->class_size);
-ti->class->interfaces = NULL;
+memcpy(ti->klass, parent->klass, parent->class_size);
+ti->klass->interfaces = NULL;
 
-for (e = parent->class->interfaces; e; e = e->next) {
+for (e = parent->klass->interfaces; e; e = e->next) {
 InterfaceClass *iface = e->data;
 ObjectClass *klass = OBJECT_CLASS(iface);
 
@@ -377,7 +377,7 @@ static void type_initialize(TypeImpl *ti)
  ti->interfaces[i].typename, parent->name);
 abort();
 }
-for (e = ti->class->interfaces; e; e = e->next) {
+for (e = ti->klass->interfaces; e; e = e->next) {
 TypeImpl *target_type = OBJECT_CLASS(e->data)->type;
 
 if (type_is_ancestor(target_type, t)) {
@@ -393,20 +393,20 @@ static void type_initialize(TypeImpl *ti)
 }
 }
 
-ti->class->properties = g_hash_table_new_full(g_str_hash, g_str_equal, 
NULL,
+ti->klass->properties = g_hash_table_new_full(g_str_hash, g_str_equal, 
NULL,
   object_property_free);
 

[PATCH 2/2] include/qom: Rename typename into type_name

2024-06-24 Thread Felix Wu
From: Roman Kiryanov 

`typename` is a C++ keyword and it prevents from
using the QEMU headers with a C++ compiler.

Google-Bug-Id: 331190993
Change-Id: Iff313ca5ec157a1a3826b4f5665073534d961a26
Signed-off-by: Felix Wu 
Signed-off-by: Roman Kiryanov 
---
 hw/core/bus.c  |   8 +--
 include/hw/qdev-core.h |   4 +-
 include/qom/object.h   |  78 +--
 qom/object.c   | 120 -
 4 files changed, 105 insertions(+), 105 deletions(-)

diff --git a/hw/core/bus.c b/hw/core/bus.c
index b9d89495cd..07c5a83673 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -152,18 +152,18 @@ static void bus_unparent(Object *obj)
 bus->parent = NULL;
 }
 
-void qbus_init(void *bus, size_t size, const char *typename,
+void qbus_init(void *bus, size_t size, const char *type_name,
DeviceState *parent, const char *name)
 {
-object_initialize(bus, size, typename);
+object_initialize(bus, size, type_name);
 qbus_init_internal(bus, parent, name);
 }
 
-BusState *qbus_new(const char *typename, DeviceState *parent, const char *name)
+BusState *qbus_new(const char *type_name, DeviceState *parent, const char 
*name)
 {
 BusState *bus;
 
-bus = BUS(object_new(typename));
+bus = BUS(object_new(type_name));
 qbus_init_internal(bus, parent, name);
 
 return bus;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 5336728a23..ede4b74bd8 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -867,9 +867,9 @@ DeviceState *qdev_find_recursive(BusState *bus, const char 
*id);
 typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
 typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
 
-void qbus_init(void *bus, size_t size, const char *typename,
+void qbus_init(void *bus, size_t size, const char *type_name,
DeviceState *parent, const char *name);
-BusState *qbus_new(const char *typename, DeviceState *parent, const char 
*name);
+BusState *qbus_new(const char *type_name, DeviceState *parent, const char 
*name);
 bool qbus_realize(BusState *bus, Error **errp);
 void qbus_unrealize(BusState *bus);
 
diff --git a/include/qom/object.h b/include/qom/object.h
index 7afdb261a8..4e69a3506d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -617,7 +617,7 @@ Object *object_new_with_class(ObjectClass *klass);
 
 /**
  * object_new:
- * @typename: The name of the type of the object to instantiate.
+ * @type_name: The name of the type of the object to instantiate.
  *
  * This function will initialize a new object using heap allocated memory.
  * The returned object has a reference count of 1, and will be freed when
@@ -625,11 +625,11 @@ Object *object_new_with_class(ObjectClass *klass);
  *
  * Returns: The newly allocated and instantiated object.
  */
-Object *object_new(const char *typename);
+Object *object_new(const char *type_name);
 
 /**
  * object_new_with_props:
- * @typename:  The name of the type of the object to instantiate.
+ * @type_name:  The name of the type of the object to instantiate.
  * @parent: the parent object
  * @id: The unique ID of the object
  * @errp: pointer to error object
@@ -673,7 +673,7 @@ Object *object_new(const char *typename);
  *
  * Returns: The newly allocated, instantiated & initialized object.
  */
-Object *object_new_with_props(const char *typename,
+Object *object_new_with_props(const char *type_name,
   Object *parent,
   const char *id,
   Error **errp,
@@ -681,7 +681,7 @@ Object *object_new_with_props(const char *typename,
 
 /**
  * object_new_with_propv:
- * @typename:  The name of the type of the object to instantiate.
+ * @type_name:  The name of the type of the object to instantiate.
  * @parent: the parent object
  * @id: The unique ID of the object
  * @errp: pointer to error object
@@ -689,7 +689,7 @@ Object *object_new_with_props(const char *typename,
  *
  * See object_new_with_props() for documentation.
  */
-Object *object_new_with_propv(const char *typename,
+Object *object_new_with_propv(const char *type_name,
   Object *parent,
   const char *id,
   Error **errp,
@@ -755,13 +755,13 @@ bool object_set_propv(Object *obj, Error **errp, va_list 
vargs);
  * object_initialize:
  * @obj: A pointer to the memory to be used for the object.
  * @size: The maximum size available at @obj for the object.
- * @typename: The name of the type of the object to instantiate.
+ * @type_name: The name of the type of the object to instantiate.
  *
  * This function will initialize an object.  The memory for the object should
  * have already been allocated.  The returned object has a reference count of 
1,
  * and will be finalized when the last reference is dropped.
  */
-void object_initialize(void *obj, size_t size, const char *typename);
+void object_ini

Re: udp guestfwd

2024-03-13 Thread Felix Wu
Hi Louai,

Are you using IPv6 or IPv4? The IPv4 is actually broken (if you want to
send multiple requests to slirp and get them forwarded).
You can check the latest comments in following tickets:
https://gitlab.freedesktop.org/slirp/libslirp/-/issues/67
https://gitlab.com/qemu-project/qemu/-/issues/1835

If you want to use IPv6, let me know and I can create pull requests in
libslirp so you can try it.

Thanks, Felix

On Fri, Dec 8, 2023 at 9:33 AM Patrick Venture  wrote:

>
> On Fri, Oct 27, 2023 at 11:44 PM Louai Al-Khanji 
> wrote:
>
>> Hi,
>>
>> I'm interested in having the guestfwd option work for udp. My
>> understanding is that currently it's restricted to only tcp.
>>
>> I'm not familiar with libslirp internals. What would need to be changed
>> to implement this? I'm potentially interested in doing the work.
>>
>> I did a tiny amount of digging around libslirp and saw this comment in
>> `udp.c':
>>
>> /*
>>  * X Here, check if it's in udpexec_list,
>>  * and if it is, do the fork_exec() etc.
>>  */
>>
>> I wonder whether that is related. In any case any help is much
>> appreciated.
>>
>
> Felix has been working in this space and it may take time to get the CLs
> landed in libslirp and qemu.
>
> Patrick
>
>>
>> Thanks,
>> Louai Al-Khanji
>>
>


[PATCH 0/1] smbios_build_type_8_table should use T8_BASE.

2024-01-11 Thread Felix Wu
It should use T8_BASE instead of T0_BASE.

Felix Wu (1):
  SMBIOS type 8 should use T8_BASE.

 hw/smbios/smbios.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.43.0.275.g3460e3d667-goog




[PATCH 1/1] SMBIOS type 8 should use T8_BASE.

2024-01-11 Thread Felix Wu
---
 hw/smbios/smbios.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 2a90601ac5..7dda84b284 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -591,6 +591,7 @@ bool smbios_skip_table(uint8_t type, bool required_table)
 #define T2_BASE 0x200
 #define T3_BASE 0x300
 #define T4_BASE 0x400
+#define T8_BASE 0x800
 #define T11_BASE 0xe00
 
 #define T16_BASE 0x1000
@@ -775,7 +776,7 @@ static void smbios_build_type_8_table(void)
 struct type8_instance *t8;
 
 QTAILQ_FOREACH(t8, &type8, next) {
-SMBIOS_BUILD_TABLE_PRE(8, T0_BASE + instance, true);
+SMBIOS_BUILD_TABLE_PRE(8, T8_BASE + instance, true);
 
 SMBIOS_TABLE_SET_STR(8, internal_reference_str, 
t8->internal_reference);
 SMBIOS_TABLE_SET_STR(8, external_reference_str, 
t8->external_reference);
-- 
2.43.0.275.g3460e3d667-goog




Re: Tips for local testing guestfwd

2023-09-06 Thread Felix Wu
Hi,
I noticed why the chardev socket backend disconnects, and I would like to
make this a RFC to see how I should fix it.
Current scenario after boot-up:

   1. tcp_chr_read_poll keeps polling the slirp_socket_can_recv, and
   slirp_socket_can_recv returns 0 since slirp_find_ctl_socket couldn't
   find the guestfwd socket.
   2. The returned 0 in step 1 was assigned to the s->max_size (s is
SocketChardev
   *), and the socket chardev handler won't read since readable size is 0.
   3. When the 1st request is sent, the guestfwd socket is added into the
   slirp's socket list, instead of 0, tcp_chr_read_poll will return the
   result of sopreprbuf > 0.
   4. tcp_chr_read reads the thing.
   5. tcp_chr_read_poll still returns things > 0, which is the output of
   sopreprbuf.
   6. tcp_chr_read reads the thing again, but there's nothing in the
   buffer, so it's unhappy, and closes the connection.
   7. any follow-up requests won't be handled.

These tcp_chr* functions are in fle [1], and slirp_* are in fle [2].

My questions:
1. Since this thing doesn't work on 2nd and later requests, I want to know
how this thing is supposed to work, and to avoid asking people vaguely, I
will provide my though following and please correct me if I am wrong:
a. The state machine in chardev socket should maintain a connected
state (s->state
== TCP_CHARDEV_STATE_CONNECTED), this means no change in [1].
b. slirp_socket_can_recv should return 0 once all data is read instead of
outcome from sopreprbuf. This means I need to remove the socket or change
its state to no file descriptor [3], namely somehow reset it.
c. When a new request comes in, it will need to add the socket back to this
slirp instance's socket list, populate its file descriptor, and establish
the connection.

b and c sounds convoluted so I want to check.

2. What is the outcome of sopreprbuf function [3]?
Since it's returned to the tcp_chr_read_poll function, I thought it's the
readable bytes in the socket, but in my test I noticed following thing:

tcp_chr_read_poll_size : s->max_size: 132480
tcp_chr_read : size: 2076
tcp_chr_read_poll_size : s->max_size: 129600
tcp_chr_read : size: 0

Even there's not remaining things in the buffer (read size 0), it's still
non-zero, and thus the read function keeps reading it until it becomes
unhappy.
Also, 132480-129600 = 2880 vs 2076, the read byte doesn't match.

Either I need to go with the way in question 1, b.c. steps, or I don't need
to delete the socket, but the sopreprbuf wasn't proper to be used there and
I need to correct it.
Also updated https://gitlab.com/qemu-project/qemu/-/issues/1835.

Any feedback will be appreciated, thanks!
Felix

[1].
https://gitlab.com/qemu-project/qemu/-/blob/master/chardev/char-socket.c#L141
[2].
https://gitlab.freedesktop.org/slirp/libslirp/-/blob/master/src/slirp.c#L1582
[3].
https://gitlab.freedesktop.org/slirp/libslirp/-/blob/master/src/socket.h#L221

On Wed, Aug 23, 2023 at 10:27 AM Felix Wu  wrote:

> Update on debugging this thing (already updated
> https://gitlab.com/qemu-project/qemu/-/issues/1835):
> I saw that `tcp_chr_free_connection` was called after the first response
> being successfully sent:
> ```
>
> slirp_guestfwd_write guestfwd_write: size 80tcp_chr_write tcp_chr_write: 
> s->state:2tcp_chr_write tcp_chr_write: len:80qemu_chr_write_parameter len: 80 
> // tracking qemu_chr_writeqemu_chr_write_res len: 80 // same 
> thingtcp_chr_free_connection tcp_chr_free_connection: state: 2, changing it 
> to disconnecttcp_chr_change_state tcp_chr_change_state: state: 2, next state: 
> 0 // state 2==connected, 0==disconnected.
>
> ```
> And after that, the state of `SocketChardev` remained disconnected, and
> when the 2nd request came in, the `tcp_chr_write` dropped it directly.
> Maybe this state machine should be reset after every connection? Not sure.
>
> On Thu, Aug 17, 2023 at 11:58 AM Felix Wu  wrote:
>
>> Hi Samuel,
>>
>> Thanks for the clarification! I missed the email so didn't reply in time,
>> but was able to figure it out.
>>
>> Hi everyone,
>> IPv6 guestfwd works in my local test but it has a weird bug: if you send
>> two requests, the first one gets the correct response, but the second one
>> gets stuck.
>> I am using a simple http server for this test, and just noticed this bug
>> also exists in IPv4 guestfwd. I've documented it in
>> https://gitlab.com/qemu-project/qemu/-/issues/1835.
>>
>> Just want to check if anyone has seen the same issue before.
>>
>> Thanks! Felix
>>
>> On Thu, Jul 20, 2023 at 7:54 AM Samuel Thibault 
>> wrote:
>>
>>> Hello,
>>>
>>> Felix Wu, le mar. 18 juil. 2023 18:12:16 -0700, a ecrit:
>>> > 02 == SYN so it looks good. B

Re: Tips for local testing guestfwd

2023-08-23 Thread Felix Wu
Update on debugging this thing (already updated
https://gitlab.com/qemu-project/qemu/-/issues/1835):
I saw that `tcp_chr_free_connection` was called after the first response
being successfully sent:
```

slirp_guestfwd_write guestfwd_write: size 80tcp_chr_write
tcp_chr_write: s->state:2tcp_chr_write tcp_chr_write:
len:80qemu_chr_write_parameter len: 80 // tracking
qemu_chr_writeqemu_chr_write_res len: 80 // same
thingtcp_chr_free_connection tcp_chr_free_connection: state: 2,
changing it to disconnecttcp_chr_change_state tcp_chr_change_state:
state: 2, next state: 0 // state 2==connected, 0==disconnected.

```
And after that, the state of `SocketChardev` remained disconnected, and
when the 2nd request came in, the `tcp_chr_write` dropped it directly.
Maybe this state machine should be reset after every connection? Not sure.

On Thu, Aug 17, 2023 at 11:58 AM Felix Wu  wrote:

> Hi Samuel,
>
> Thanks for the clarification! I missed the email so didn't reply in time,
> but was able to figure it out.
>
> Hi everyone,
> IPv6 guestfwd works in my local test but it has a weird bug: if you send
> two requests, the first one gets the correct response, but the second one
> gets stuck.
> I am using a simple http server for this test, and just noticed this bug
> also exists in IPv4 guestfwd. I've documented it in
> https://gitlab.com/qemu-project/qemu/-/issues/1835.
>
> Just want to check if anyone has seen the same issue before.
>
> Thanks! Felix
>
> On Thu, Jul 20, 2023 at 7:54 AM Samuel Thibault 
> wrote:
>
>> Hello,
>>
>> Felix Wu, le mar. 18 juil. 2023 18:12:16 -0700, a ecrit:
>> > 02 == SYN so it looks good. But both tcpdump and wireshark (looking
>> into packet
>> > dump provided by QEMU invocation)
>>
>> Which packet dump?
>>
>> > I added multiple prints inside slirp and confirmed the ipv6 version of
>> [1] was
>> > reached.
>> > in tcp_output function [2], I got following print:
>> > qemu-system-aarch64: info: Slirp: AF_INET6 out dst ip =
>> > fdb5:481:10ce:0:8c41:aaff:fea9:f674, port = 52190
>> > qemu-system-aarch64: info: Slirp: AF_INET6 out src ip = fec0::105, port
>> = 54322
>> > It looks like there should be something being sent back to the guest,
>>
>> That's what it is.
>>
>> > unless my understanding of tcp_output is wrong.
>>
>> It looks so.
>>
>> > To understand the datapath of guestfwd better, I have the following
>> questions:
>> > 1. What's the meaning of tcp_input and tcp_output? My guess is the
>> following
>> > graph, but I would like to confirm.
>>
>> No, tcp_input is for packets that come from the guest, and tcp_output is
>> for packets that are send to the guest. So it's like that:
>>
>> > tcp_inputwrite_cb  host send()
>> > QEMU > slirp ---> QEMU > host
>> > <<- <-
>> >  tcp_output  slirp_socket_recvhost recv()
>>
>> > 2. I don't see port 6655 in the above process. How does slirp know 6655
>> is the
>> > port that needs to be visited on the host side?
>>
>> Slirp itself *doesn't* know that port. The guestfwd piece just calls the
>> SlirpWriteCb when it has data coming from the guest. See the
>> documentation:
>>
>> /* Set up port forwarding between a port in the guest network and a
>>  * callback that will receive the data coming from the port */
>> SLIRP_EXPORT
>> int slirp_add_guestfwd(Slirp *slirp, SlirpWriteCb write_cb, void *opaque,
>>struct in_addr *guest_addr, int guest_port);
>>
>> and
>>
>> /* This is called by the application for a guestfwd, to provide the data
>> to be
>>  * sent on the forwarded port */
>> SLIRP_EXPORT
>> void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr, int
>> guest_port,
>>const uint8_t *buf, int size);
>>
>> Samuel
>>
>


Re: Tips for local testing guestfwd

2023-08-17 Thread Felix Wu
Hi Samuel,

Thanks for the clarification! I missed the email so didn't reply in time,
but was able to figure it out.

Hi everyone,
IPv6 guestfwd works in my local test but it has a weird bug: if you send
two requests, the first one gets the correct response, but the second one
gets stuck.
I am using a simple http server for this test, and just noticed this bug
also exists in IPv4 guestfwd. I've documented it in
https://gitlab.com/qemu-project/qemu/-/issues/1835.

Just want to check if anyone has seen the same issue before.

Thanks! Felix

On Thu, Jul 20, 2023 at 7:54 AM Samuel Thibault 
wrote:

> Hello,
>
> Felix Wu, le mar. 18 juil. 2023 18:12:16 -0700, a ecrit:
> > 02 == SYN so it looks good. But both tcpdump and wireshark (looking into
> packet
> > dump provided by QEMU invocation)
>
> Which packet dump?
>
> > I added multiple prints inside slirp and confirmed the ipv6 version of
> [1] was
> > reached.
> > in tcp_output function [2], I got following print:
> > qemu-system-aarch64: info: Slirp: AF_INET6 out dst ip =
> > fdb5:481:10ce:0:8c41:aaff:fea9:f674, port = 52190
> > qemu-system-aarch64: info: Slirp: AF_INET6 out src ip = fec0::105, port
> = 54322
> > It looks like there should be something being sent back to the guest,
>
> That's what it is.
>
> > unless my understanding of tcp_output is wrong.
>
> It looks so.
>
> > To understand the datapath of guestfwd better, I have the following
> questions:
> > 1. What's the meaning of tcp_input and tcp_output? My guess is the
> following
> > graph, but I would like to confirm.
>
> No, tcp_input is for packets that come from the guest, and tcp_output is
> for packets that are send to the guest. So it's like that:
>
> > tcp_inputwrite_cb  host send()
> > QEMU > slirp ---> QEMU > host
> > <<- <-
> >  tcp_output  slirp_socket_recvhost recv()
>
> > 2. I don't see port 6655 in the above process. How does slirp know 6655
> is the
> > port that needs to be visited on the host side?
>
> Slirp itself *doesn't* know that port. The guestfwd piece just calls the
> SlirpWriteCb when it has data coming from the guest. See the
> documentation:
>
> /* Set up port forwarding between a port in the guest network and a
>  * callback that will receive the data coming from the port */
> SLIRP_EXPORT
> int slirp_add_guestfwd(Slirp *slirp, SlirpWriteCb write_cb, void *opaque,
>struct in_addr *guest_addr, int guest_port);
>
> and
>
> /* This is called by the application for a guestfwd, to provide the data
> to be
>  * sent on the forwarded port */
> SLIRP_EXPORT
> void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr, int
> guest_port,
>const uint8_t *buf, int size);
>
> Samuel
>


Re: Tips for local testing guestfwd

2023-07-18 Thread Felix Wu
Hi all,

I am continuing debugging the ipv6 guestfwd feature, and I would like to
understand the behavior of slirp better.

Progress I've made:
Let QEMU take parameter like following:
guestfwd=tcp:[fec0::105]:54322-tcp:[::1]:6655
For slirp side, I basically searched for the appearance of gfwd_list and
made all code traverse the fwd list compatible with ipv6.
With these change, now I can see the packets coming out of the guest OS to
the assigned guest server port via tcpdump:
```
00:38:18.831831 IP6 fdb5:481:10ce:0:8c41:aaff:fea9:f674.52190 >
fec0::105.54322: tcp 0
0x:  600a 1f94 0028 0640 fdb5 0481 10ce   `(.@
0x0010:  8c41 aaff fea9 f674 fec0     .A.t
0x0020:     0105 cbde d432 df6d 8332  ...2.m.2
0x0030:    a0*02* fd20 535f  0204 05a0  S_..
0x0040:  0402 080a b87b fd3b   0103 0307  .{.;
```
02 == SYN so it looks good. But both tcpdump and wireshark (looking into
packet dump provided by QEMU invocation) didn't see any response and this
packet never reached the host.
I added multiple prints inside slirp and confirmed the ipv6 version of [1]
was reached.
in tcp_output function [2], I got following print:
qemu-system-aarch64: info: Slirp: AF_INET6 out dst ip =
fdb5:481:10ce:0:8c41:aaff:fea9:f674, port = 52190
qemu-system-aarch64: info: Slirp: AF_INET6 out src ip = fec0::105, port =
54322
It looks like there should be something being sent back to the guest,
unless my understanding of tcp_output is wrong.

To understand the datapath of guestfwd better, I have the following
questions:
1. What's the meaning of tcp_input and tcp_output? My guess is the
following graph, but I would like to confirm.
   tcp_input tcp_output
QEMU > slirp --> host
<   <--
 tcp_output   tcp_input

2. I don't see port 6655 in the above process. How does slirp know 6655 is
the port that needs to be visited on the host side?

Thanks in advance, Felix
[1].
https://gitlab.freedesktop.org/slirp/libslirp/-/blob/master/src/tcp_input.c#L630
[2].
https://gitlab.freedesktop.org/slirp/libslirp/-/blob/master/src/tcp_output.c#L477


On Mon, Jun 26, 2023 at 3:08 AM Samuel Thibault 
wrote:

> Hello,
>
> Felix Wu  wrote:
> > 2. I want to understand what ip I should use. Currently I have following
> > formats for the QEMU invocation in ipv6:
> > ```
> > guestfwd=tcp:[::1]:1234-tcp:[my:host:ip:from:ifconfig]:22
> > ```
> > I know the general form is `guestfwd=tcp:server:port-dev`, where
> > server:port is for guest,
>
> Yes, the address to be used within the guest network. So it needs to be
> within the guest network.
>
> > Is the aforementioned invocation correct?
>
> No, because ::1 isn't in the guest network.
>
> > Or in this case [::1] is the local host address and I should put qemu
> > address for it instead?
>
> You can use whatever IP you want, as long as it's in the guest network.
> e.g. [fec0::1234] if you're with the default fec0::/64 network.
>
> > 3. Is there a default ipv6 address for QEMU instance? I think I need it
> in
> > the invocation.
>
> By default it's address 2 within the prefix, i.e. fec0::2 with the
> default fec0::/64 network.
>
> Samuel
>


Tips for local testing guestfwd

2023-06-25 Thread Felix Wu
Hi all,

TL,DR: I am working on QEMU ipv6 guestfwd feature and finished coding, and
would like to learn the best practice to test it.
Context: in slirp side this task is tracking by [1].
Currently, I have done following:
i. made char parse + guestfwd functions happy with ipv6 address.
ii. enabled debug print and made sure the ip and port are inserted into the
forward list in libslirp.
To sufficiently verify it's working, I do have three questions:
1. I want to forward a port in the guest (OS in QEMU) to a port 22 on the
host OS, and ssh from guest back to host,
does this sound reasonable? If this is not a good idea, what method is
recommended?
2. I want to understand what ip I should use. Currently I have following
formats for the QEMU invocation in ipv6:
```
guestfwd=tcp:[::1]:1234-tcp:[my:host:ip:from:ifconfig]:22
```
I know the general form is `guestfwd=tcp:server:port-dev`, where
server:port is for guest, dev is for host.
Adding [] in my implementation will let QEMU know it's ipv6.
Is the aforementioned invocation correct? Or in this case [::1] is the
local host address and I should put qemu address
for it instead?
3. Is there a default ipv6 address for QEMU instance? I think I need it in
the invocation.

Thanks in advance! Felix

[1]. https://gitlab.freedesktop.org/slirp/libslirp/-/issues/67