Re: mbuf_outbound_finalize bug?
Re: mbuf_outbound_finalize bug?
- Subject: Re: mbuf_outbound_finalize bug?
- From: Terry Lambert <email@hidden>
- Date: Mon, 1 Jan 2007 17:28:58 -0800
Josh G. is the expert here, but I'm pretty sure that you would end up
shooting your foot off if you did this.
When you set these flags, they are generally done in the protocol
specific output functions, and since TCP and UDP are both layered on
top of IP, you are requesting that the checksum be calculated both as
if the packet were a TCP and a UDP packet, and then asking for the IP
encapsulation checksum to also be calculated.
If you had hardware checksum assist (common on most hardware these
days), then you may end up with both the TCP and UDP checksums
calculated, and one of them stomping the other.
FWIW, csum_data is a value, not a pointer, for the 16 bit checksum
that's trying to be calculated, and is used as an accumulator for the
checksum calculation. Practically, this means that for your
applications, you'd likely want to leave it alone, unless you were
setting flags requesting checksums. If you were doing that, then
you'd need to know if it was on input or output, and since you are
doing a full checksum, then you'd want to set it to 0xffff or 0,
respectively (see the source code for the protocol specific input/
output mechansims for details.
Again, if you are doing an incremental update (i.e. you already have a
valid packet in hand, and are trying to replace a few data fields in
it without hanging packet size), then the only things you will be
touching are the data, the layer 4 checksump (TCP or UDP) and the
layer 3 checksum (IP). You won't be setting these flags to request
anything, clearing them to avoid requesting anything, and you won't be
using the accumulator yourself.
I think at this point, it would be educational to download the xnu
sources and do a "grep -r" for the field names and flags to better
understand how the code works.
-- Terry
On Dec 29, 2006, at 9:31 AM, Bhavesh Davda wrote:
Hi Terry,
That's exactly what I ended up implementing. I haven't read RFC 1624,
but I will.
Related question: if ever csum_flags has both CSUM_IP and CSUM_TCP |
CSUM_UDP set, then what is csum_data going to point to?
Thanks!
- Bhavesh
On 12/28/06, Terry Lambert <email@hidden> wrote:
Are you changing the length of the packet, or only the contents?
If you are only changing the contents, then I suggest an mbuf-chain
aware incremental checksum update as an alternative to copying it
to a
contiguous buffer, operating on it, converting it to mbufs, then
doing
a full software checksum of the whole packet.
See RFC 1624.
-- Terry
On Dec 22, 2006, at 10:12 AM, Adi Masputra wrote:
> On Dec 22, 2006, at 9:58 AM, Bhavesh Davda wrote:
>>
>> I compare the result of mbuf_pkthdr_len agains the network-byte-
order
>> value of IP length on purpose, to know whether indeed the value
of IP
>> length in the packet is *not* in host-byte-order, as I thought you
>> said mbuf_outbound_finalize() expects.
>
> Since your module is an interface filter, the field(s) in the
> header(s)
> will be in network byte-order. All you need to do is to byte-swap
> the length at all times assuming it is an IPv4 packet.
>
>>
>> Oh boy. This is going to get quite expensive to do on a per packet
>> basis. I thought I could simply get a pointer into the packet
payload
>> to the IP length field in the IP header, and do a memory read/
write,
>> rather than going through so many function calls and memcpy's,
albeit
>> small (2 byte) ones.
>
> Well, you are already requesting the stack to perform software
> checksum
> calculation on the packet which includes the transport payload; in
> terms
> of per-byte cost that is much more significant than traversing the
> mbuf
> chain to safely obtain the 2-octets IP length in the header. :)
>
>>
>> Is there any other way to do it more optimally? I understand your
>> concern about non-linear mbufs, and not assuming that the IP
header
>> is
>> in the pkthdr mbuf. But realistically, is that ever going to be
the
>> case?
>>
>
> There's no guarantee that the headers will be in contiguous span
> although for the majority of cases they would. Sorry, but as I
said
> the fix is already in order.
>
> Adi
>
>> Thanks!
>>
>> --
>> Bhavesh P. Davda
>
> _______________________________________________
> Do not post admin requests to the list. They will be ignored.
> Darwin-kernel mailing list (email@hidden)
> Help/Unsubscribe/Update your Subscription:
>
> This email sent to email@hidden
--
Bhavesh P. Davda
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Darwin-kernel mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden