Date: Sun, 28 Nov 2004 01:00:00 +0100 (CET) From: Sten Spans <sten@blinkenlights.nl> To: Robert Watson <rwatson@FreeBSD.org>, John Baldwin <jhb@FreeBSD.org> Cc: freebsd-alpha@FreeBSD.org Subject: Re: alpha and em mtu Message-ID: <Pine.SOC.4.61.0411280044200.25496@tea.blinkenlights.nl> In-Reply-To: <200411231117.35467.jhb@FreeBSD.org> References: <Pine.SOC.4.61.0411142153430.26307@tea.blinkenlights.nl> <Pine.SOC.4.61.0411230259030.10997@tea.blinkenlights.nl> <Pine.SOC.4.61.0411230406010.10997@tea.blinkenlights.nl> <200411231117.35467.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
---559023410-851401618-1101600000=:25496
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed
On Tue, 23 Nov 2004, John Baldwin wrote:
> On Monday 22 November 2004 10:09 pm, Sten Spans wrote:
>> On Tue, 23 Nov 2004, Sten Spans wrote:
>>>> doesn't seem to print anything, but ...
>>>>
>>>> if_em.c
>>>> 2442
>>>> 2443 /*if (ifp->if_mtu <= ETHERMTU) { */
>>>> 2444 m_adj(mp, ETHER_ALIGN);
>>>> 2445 /*} */
>>>> 2446
>>>>
>>>> does seem to fix the crash, also trashes the performance,
>>>> but that's another matter. It looks like mbuf alignment is
>>>> needed, if_bge seems to provide reasonable examples.
>>>
>>> And looking at netbsd/openbsd clarifies the whole issue,
>>>
>>> #ifdef __STRICT_ALIGNMENT
>>> /*
>>> * The ethernet payload is not 32-bit aligned when
>>> * Jumbo packets are enabled, so on architectures
>>> with
>>> * strict alignment we need to shift the entire
>>> packet
>>> * ETHER_ALIGN bytes. Ugh.
>>> */
>>>
>>>
>>> This diff probably should be merged.
>>> http://www.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pci/if_em.c.diff?r1=1.2
>>> 2&r2=1.23
>>>
>>> Although I don't know wether freebsd has the STRICT_ALIGNMENT define.
>>
>> This is an initial patch based on the openbsd code,
>> which solves the if_em issue, and seems to give ok performance
>> ( note to self: turn off debugging when testing network performance ).
>>
>> Comments welcome.
>
> We do need some kind of __STRICT_ALIGNMENT macro I think if we don't already
> have one as archs other than i386 might not need the alignment.
bge needs some fixing as well then:
2773 #ifndef __i386__
2774 /*
2775 * The i386 allows unaligned accesses, but for other
2776 * platforms we must make sure the payload is aligned.
2777 */
2778 if (sc->bge_rx_alignment_bug) {
2779 bcopy(m->m_data, m->m_data + ETHER_ALIGN,
2780 cur_rx->bge_len);
2781 m->m_data += ETHER_ALIGN;
2782 }
2783 #endif
Not sure if this a better fix for the same problem...
> At the least
> there could be a block near the top of if_em.h that was:
>
> #if defined(__alpha__)
> #define STRICT_ALIGNMENT
> #endif
>
> and other architectures could be fixed by simply adding another
> 'defined(__foo__)' clause without having to change the ifdef and comments
> down in the code itself.
I've attatched an updated version of the patch, with your comment
included.
> As to the correctness of the em(4) change,
> hopefully Robert can speak to that.
Have you had time to look at this robert ?
--
Sten Spans
"There is a crack in everything, that's how the light gets in."
Leonard Cohen - Anthem
---559023410-851401618-1101600000=:25496
Content-Type: TEXT/PLAIN; charset=US-ASCII; name="if_em-alpha.diff"
Content-Transfer-Encoding: BASE64
Content-ID: <Pine.SOC.4.61.0411280100000.25496@tea.blinkenlights.nl>
Content-Description:
Content-Disposition: attachment; filename="if_em-alpha.diff"
LS0tIGlmX2VtLmMub3JpZwlUdWUgTm92IDIzIDAzOjAzOjIzIDIwMDQNCisr
KyBpZl9lbS5jCVN1biBOb3YgMjggMDA6NTM6NTEgMjAwNA0KQEAgLTI3ODQs
OCArMjc4NCw1NyBAQA0KIAkJCS8qIEFzc2lnbiBjb3JyZWN0IGxlbmd0aCB0
byB0aGUgY3VycmVudCBmcmFnbWVudCAqLw0KIAkJCW1wLT5tX2xlbiA9IGxl
bjsNCiANCisjaWZuZGVmIF9fU1RSSUNUX0FMSUdOTUVOVA0KKwkJCS8qDQor
CQkJICogVGhlIGV0aGVybmV0IHBheWxvYWQgaXMgbm90IDMyLWJpdCBhbGln
bmVkIHdoZW4NCisJCQkgKiBKdW1ibyBwYWNrZXRzIGFyZSBlbmFibGVkLCBz
byBvbiBhcmNoaXRlY3R1cmVzIHdpdGgNCisJCQkgKiBzdHJpY3QgYWxpZ25t
ZW50IHdlIG5lZWQgdG8gc2hpZnQgdGhlIGVudGlyZSBwYWNrZXQNCisJCQkg
KiBFVEhFUl9BTElHTiBieXRlcy4gVWdoLg0KKwkJCSAqLw0KKwkJCWlmIChp
ZnAtPmlmX210dSA+IEVUSEVSTVRVKSB7DQorCQkJCXVuc2lnbmVkIGNoYXIg
dG1wX2FsaWduX2J1ZltFVEhFUl9BTElHTl07DQorCQkJCWludCB0bXBfYWxp
Z25fYnVmX2xlbiA9IDA7DQorDQorCQkJCWlmIChwcmV2X2xlbl9hZGogPiBh
ZGFwdGVyLT5hbGlnbl9idWZfbGVuKQ0KKwkJCQkJcHJldl9sZW5fYWRqIC09
IGFkYXB0ZXItPmFsaWduX2J1Zl9sZW47DQorCQkJCWVsc2UNCisJCQkJCXBy
ZXZfbGVuX2FkaiA9IDA7DQorDQorCQkJCWlmIChtcC0+bV9sZW4gPiBNQ0xC
WVRFUyAtIEVUSEVSX0FMSUdOKSB7DQorCQkJCQliY29weShtcC0+bV9kYXRh
ICsNCisJCQkJCSAgICAoTUNMQllURVMgLSBFVEhFUl9BTElHTiksDQorCQkJ
CQkgICAgJnRtcF9hbGlnbl9idWYsDQorCQkJCQkgICAgRVRIRVJfQUxJR04p
Ow0KKwkJCQkJdG1wX2FsaWduX2J1Zl9sZW4gPSBtcC0+bV9sZW4gLQ0KKwkJ
CQkJICAgIChNQ0xCWVRFUyAtIEVUSEVSX0FMSUdOKTsNCisJCQkJCW1wLT5t
X2xlbiAtPSBFVEhFUl9BTElHTjsNCisJCQkJfSANCisNCisJCQkJaWYgKG1w
LT5tX2xlbikgew0KKwkJCQkJYmNvcHkobXAtPm1fZGF0YSwNCisJCQkJCSAg
ICBtcC0+bV9kYXRhICsgRVRIRVJfQUxJR04sDQorCQkJCQkgICAgbXAtPm1f
bGVuKTsNCisJCQkJCWlmICghYWRhcHRlci0+YWxpZ25fYnVmX2xlbikNCisJ
CQkJCQltcC0+bV9kYXRhICs9IEVUSEVSX0FMSUdOOw0KKwkJCQl9DQorDQor
CQkJCWlmIChhZGFwdGVyLT5hbGlnbl9idWZfbGVuKSB7DQorCQkJCQltcC0+
bV9sZW4gKz0gYWRhcHRlci0+YWxpZ25fYnVmX2xlbjsNCisJCQkJCWJjb3B5
KCZhZGFwdGVyLT5hbGlnbl9idWYsDQorCQkJCQkgICAgbXAtPm1fZGF0YSwN
CisJCQkJCSAgICBhZGFwdGVyLT5hbGlnbl9idWZfbGVuKTsNCisJCQkJfQ0K
Kw0KKwkJCQlpZiAodG1wX2FsaWduX2J1Zl9sZW4pIA0KKwkJCQkJYmNvcHko
JnRtcF9hbGlnbl9idWYsDQorCQkJCQkgICAgJmFkYXB0ZXItPmFsaWduX2J1
ZiwNCisJCQkJCSAgICB0bXBfYWxpZ25fYnVmX2xlbik7DQorCQkJCWFkYXB0
ZXItPmFsaWduX2J1Zl9sZW4gPSB0bXBfYWxpZ25fYnVmX2xlbjsNCisJCQl9
DQorI2VuZGlmIC8qIF9fU1RSSUNUX0FMSUdOTUVOVCAqLw0KKw0KIAkJCWlm
IChhZGFwdGVyLT5mbXAgPT0gTlVMTCkgew0KLQkJCQltcC0+bV9wa3RoZHIu
bGVuID0gbGVuOw0KKwkJCQltcC0+bV9wa3RoZHIubGVuID0gbXAtPm1fbGVu
Ow0KIAkJCQlhZGFwdGVyLT5mbXAgPSBtcDsJIC8qIFN0b3JlIHRoZSBmaXJz
dCBtYnVmICovDQogCQkJCWFkYXB0ZXItPmxtcCA9IG1wOw0KIAkJCX0gZWxz
ZSB7DQpAQCAtMjgwMSw3ICsyODUwLDcgQEANCiAJCQkJfQ0KIAkJCQlhZGFw
dGVyLT5sbXAtPm1fbmV4dCA9IG1wOw0KIAkJCQlhZGFwdGVyLT5sbXAgPSBh
ZGFwdGVyLT5sbXAtPm1fbmV4dDsNCi0JCQkJYWRhcHRlci0+Zm1wLT5tX3Br
dGhkci5sZW4gKz0gbGVuOw0KKwkJCQlhZGFwdGVyLT5mbXAtPm1fcGt0aGRy
LmxlbiArPSBtcC0+bV9sZW47DQogCQkJfQ0KIA0KICAgICAgICAgICAgICAg
ICAgICAgICAgIGlmIChlb3ApIHsNCi0tLSBpZl9lbS5oLm9yaWcJV2VkIE5v
diAxMCAwMzoxODo1MiAyMDA0DQorKysgaWZfZW0uaAlTdW4gTm92IDI4IDAw
OjUzOjEzIDIwMDQNCkBAIC03OSw2ICs3OSwxMCBAQA0KIA0KICNpbmNsdWRl
IDxkZXYvZW0vaWZfZW1faHcuaD4NCiANCisjaWYgZGVmaW5lZChfX2FscGhh
X18pDQorI2RlZmluZQkJX19TVFJJQ1RfQUxJR05NRU5UDQorI2VuZGlmDQor
DQogLyogVHVuYWJsZXMgKi8NCiANCiAvKg0KQEAgLTM0Niw2ICszNTAsMTIg
QEANCiAJaW50ICAgICAgICAgICAgIGlvX3JpZDsNCiAJdV9pbnQ4X3QgICAg
ICAgIHVuaXQ7DQogCXN0cnVjdCBtdHgJbXR4Ow0KKw0KKyNpZm5kZWYgX19T
VFJJQ1RfQUxJR05NRU5UDQorCS8qIFVzZWQgZm9yIGNhcnJ5aW5nIGZvcndh
cmQgYWxpZ25tZW50IGFkanVzdG1lbnRzICovDQorCXVuc2lnbmVkIGNoYXIJ
YWxpZ25fYnVmW0VUSEVSX0FMSUdOXTsJLyogdGFpbCBvZiB1bmFsaWduZWQg
cGFja2V0ICovDQorCXVfaW50OF90CWFsaWduX2J1Zl9sZW47CQkvKiBieXRl
cyBpbiB0YWlsICovDQorI2VuZGlmIC8qIF9fU1RSSUNUX0FMSUdOTUVOVCAq
Lw0KIA0KIAkvKiBJbmZvIGFib3V0IHRoZSBib2FyZCBpdHNlbGYgKi8NCiAJ
dV9pbnQzMl90ICAgICAgIHBhcnRfbnVtOw0K
---559023410-851401618-1101600000=:25496--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.SOC.4.61.0411280044200.25496>