Re: UDP checksum broken, -head and releng_8

On Sat, 8 Jan 2011, Bjoern A. Zeeb wrote:

> On Sat, 8 Jan 2011, Daniel Eischen wrote:
>
>>>>>> When sending multicast packets to a socket that is _not_
>>>>>> bound to the multicast address, this generates bad UDP
>>>>>> checksums.  This use to work and was broke sometime between
>>>>>> the middle of October and late December as far as I can
>>>>>> tell.
>>>>> 
>>>>> My very best guess would be: r215110
>>>> 
>>>> It doesn't look very harmful, but I'll try backing it out.
>>> 
>>> Backing this out seems to fix it.  I'll have to test it
>>> more after I get some sleep ;-)
>> 
>> I've attached what may be a proper patch.  Please review.
>> I'd like to get this fixed in releng_8 too.
>
> I would remove the comment from the MC good path about the in_pcbladdr()
> error but just change the description at the top s,use,prefer, to be
> more exact.

Agreed.

> The other question I am not sure what shoud happen is, in the case
> in_pcbladdr() returns a source address but a given one via MC option/ifp
> does not find an address (in case outgoing itnerface could be different)?
> That was never considered in the past.

Yes, I considered that as well.  I wasn't sure about that,
but the more I think about it, it might make sense to ignore
any error from an invalid/nonexistent interface set as
an MC option if we are able to find one via in_pcbladdr().
But we'll ignore that for now.

Also, are there any restrictions with jail?  If we're
in a jail and can't find an address, do we really want
to allow _any_ address set with an MC option?  I've
never used jails, but was just wondering if the
application could somehow use an interface address
that it wasn't allowed to use.

> It's probably easiest understood with the slightly modified version
> of the patch.
>
> Otherwise I think it would match both the historic behaviour again
> and keep the fix for r215110 (that rev. should be mentioned in the
> commit message).
>
> So apart from the 1 line comment change (ignoring my XXX-BZ completely
> for the moment and this fix) this looks good.

Ok, I'll commit the patch, and thank you very much
for helping me solve this problem :-)

-- 
DE