Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host

[Blue Swirl]

On Sat, Aug 6, 2011 at 1:33 PM, Anthony Liguori <address@hidden> wrote:
> On 08/05/2011 10:25 PM, TeLeMan wrote:
>>
>> On Sat, Aug 6, 2011 at 04:46, Blue Swirl<address@hidden>  wrote:
>>>
>>> On Fri, Aug 5, 2011 at 8:09 PM, Kenneth Salerno
>>> <address@hidden>  wrote:
>>>>
>>>> Hi,
>>>>
>>>> I'm not sure if any defaults (build or runtime) have changed since
>>>> 0.14.1, but I can no longer get the following to work anymore for QEMU
>>>> versions 0.15.0-rc2 or recent development builds:
>>>>
>>>>  -device e1000,netdev=mynet0 -netdev type=user,id=mynet0 ...
>>>>
>>>> Works great in 0.14.1 however.
>>>>
>>>>  From the QEMU console, "info networking" shows the NIC e1000 and the
>>>> VLAN correctly setup, the guest (RHEL 6.1 x86_64) has its NIC recognized 
>>>> and
>>>> networking setup, just can't seem to communicate with the gateway
>>>> (10.0.2.2). The only difference I see in the console is cosmetic
>>>> (restricted=off rather than restricted=n).
>>>>
>>>> Host OS: Windows XP
>>>> Build env: i686-pc-mingw32-gcc 4.5.2, binutils 2.21.53.20110731 i386pe
>>>> Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14, mingw32-glib 2.28.1-1,
>>>>             mingw32-gettext 0.18.1-2
>>>> Guest OS: RHEL 6.1
>>>>
>>>> Is it just me?
>>>
>>> No, this is fallout from glib use:
>>> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00134.html
>>>
>>> The fix is to rewrite structures without using GCC bit fields.
>>
>> -mms-bitfields affects all byte-alignments in a structure. For example,
>> struct s
>> {
>>    uint8_t a;
>>    uint32_t b;
>> } __attribute__((packed));
>>
>> sizeof(s) is 5 without -mms-bitfields but sizeof(s) is 8 with
>> -mms-bitfields.
>
> If you can identify the offending structs, you can do:
>
> #pragma pack(push,1)
>
> struct s
> {
>    uint8_t a;
>    uint32_t b;
> } __attribute__((packed));
>
> #pragma pack(pop)

I grepped the tree for ((packed)). The only two places where bit
fields are used with packed structs are in SLIRP:

struct ip {
#ifdef HOST_WORDS_BIGENDIAN
        u_int ip_v:4,                   /* version */
                ip_hl:4;                /* header length */
#else
        u_int ip_hl:4,          /* header length */
                ip_v:4;                 /* version */
#endif
        uint8_t         ip_tos;                 /* type of service */
        uint16_t        ip_len;                 /* total length */
        uint16_t        ip_id;                  /* identification */
        uint16_t        ip_off;                 /* fragment offset field */
#define IP_DF 0x4000                    /* don't fragment flag */
#define IP_MF 0x2000                    /* more fragments flag */
#define IP_OFFMASK 0x1fff               /* mask for fragmenting bits */
        uint8_t ip_ttl;                 /* time to live */
        uint8_t ip_p;                   /* protocol */
        uint16_t        ip_sum;                 /* checksum */
        struct  in_addr ip_src,ip_dst;  /* source and dest address */
} __attribute__((packed));

struct  ip_timestamp {
        uint8_t ipt_code;               /* IPOPT_TS */
        uint8_t ipt_len;                /* size of structure (variable) */
        uint8_t ipt_ptr;                /* index of current entry */
#ifdef HOST_WORDS_BIGENDIAN
        u_int   ipt_oflw:4,             /* overflow counter */
                ipt_flg:4;              /* flags, see below */
#else
        u_int   ipt_flg:4,              /* flags, see below */
                ipt_oflw:4;             /* overflow counter */
#endif
        union ipt_timestamp {
                n_long  ipt_time[1];
                struct  ipt_ta {
                        struct in_addr ipt_addr;
                        n_long ipt_time;
                } ipt_ta[1];
        } ipt_timestamp;
} __attribute__((packed));

I'd avoid the bit fields altogether in both cases, then also the
#ifdeffery could be removed.