Opened 8 years ago

Closed 8 years ago

#7611 closed bug (fixed)

[ipro1000] is broken in VirtualBox since r41815

Reported by: diver Owned by: nobody
Priority: normal Milestone: R1
Component: Drivers/Network/ipro1000 Version: R1/Development
Keywords: Cc: modeenf
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Intel PRO/1000 MT Desktop is not detected anymore since hrev41815. It isn't listed in ifconfig output.

Change History (19)

comment:1 by tonestone57, 8 years ago

FreeBSD drivers (like this one) are meant to be imported only without any changes to code or name.

This driver's files were changed which is wrong. Only glue code (glue.c) should be changed by Haiku developer. All BSD network (C & Header) files should not be changed in anyway. Import newer versions and use them.

These changes will get reverted for that fact alone.

comment:2 by modeenf, 8 years ago

Sorry for breaking things.. I implemented the same changes as the previous developer did in his code. Are there any way of removing defines true and false from a file from within the glue.c code alone? (e1000_osdep.h in this case)

Also there was a lot of "int 1 = 0" in for loops in the if_em.c file that would not compile if not moved out of the loop.

so changes was e1000_osdep.h e1000_osdep.c if_em.c

Going to see what else when I'm getting home..

comment:3 by modeenf, 8 years ago

Cc: modeenf added

in reply to:  1 ; comment:4 by axeld, 8 years ago

Replying to tonestone57:

FreeBSD drivers (like this one) are meant to be imported only without any changes to code or name.

That's not entirely correct; while this would be the perfect aim to just to that, sometimes it's just not possible, since FreeBSD has a different interrupt model that can only be worked around using the glue code to a certain extent. In this case, if I understood modeenf correctly, he had to work around a few ISO-C99 issues which is not supported by GCC2.

While modeenf did not really work as clean as he should have (as pointed out by Michael on the mailing list), and as he was told to before having done anything, too (by myself), if the card is not detected anymore, it might actually be that the original driver dropped support for it; otherwise it is of course possible that modeenf messed something up, but that shouldn't be implied :-)

Can someone add the exact PCI device ID of that card to this ticket?

comment:5 by modeenf, 8 years ago

hm yes sorry for that. What I did was copy files from a clean freebsd directory (the one i will add to the vendor when i have my new building system up, the old one only hade trunk)

So where would i place the files in haiku/vendor/freebsd? Perhaps it also needs to be cleaned? ;)

There are actually some errors reported in freebsd 8.1 but they are about not getting an IP adress.

in reply to:  4 ; comment:6 by siarzhuk, 8 years ago

Replying to axeld:

Can someone add the exact PCI device ID of that card to this ticket?

The one from following:

device Network controller (Ethernet controller) [2|0|0]
  vendor 8086: Intel Corporation
  device 100e: 82540EM Gigabit Ethernet Controller

[...]
 
device Network controller (Ethernet controller) [2|0|0]
  vendor 8086: Intel Corporation
  device 1004: 82543GC Gigabit Ethernet Controller (Copper)

[...]

device Network controller (Ethernet controller) [2|0|0]
  vendor 8086: Intel Corporation
  device 100f: 82545EM Gigabit Ethernet Controller (Copper)

in reply to:  6 comment:7 by modeenf, 8 years ago

Replying to siarzhuk:

Replying to axeld:

Can someone add the exact PCI device ID of that card to this ticket?

The one from following:

[...] They are all in the source..

}}}

comment:8 by modeenf, 8 years ago

The only two things I can imagine it is. Are ether em_sysctl_reg_handler was needed (if_em.c) or some define are missing.

comment:9 by axeld, 8 years ago

Since VirtualBox is available for a lot of platforms, can you just install it, and look into the issue? If VirtualBox is broken, the chance is high that lots of other cards are broken as well.

As Michael already pointed out, the sysctl handler has no effect on Haiku whatsoever.

comment:10 by modeenf, 8 years ago

That I can do :). Aren't the same card used in Vmware also?

comment:11 by modeenf, 8 years ago

I have now tested vmware’s version of e1000 the 3 version in VirtualBox and all of them shows up as a device.

As I was on the train I didn’t have any network connection so no connection was tested only to see them in Network pref.

Testad with haiku-r1a3-rc-hrev41843-vmware image and VirtualBox 4.0.8 hrev71778 in Windows 7.

Can you do some more testing?

in reply to:  11 ; comment:12 by augiedoggie, 8 years ago

Replying to modeenf:

Testad with haiku-r1a3-rc-hrev41843-vmware image and VirtualBox 4.0.8 hrev71778 in Windows 7.

You tested with an A3 image but I don't believe that change was merged into that branch. I can confirm that trunk builds do not work properly.

in reply to:  12 comment:13 by modeenf, 8 years ago

Replying to augiedoggie:

Replying to modeenf:

Testad with haiku-r1a3-rc-hrev41843-vmware image and VirtualBox 4.0.8 hrev71778 in Windows 7.

You tested with an A3 image but I don't believe that change was merged into that branch. I can confirm that trunk builds do not work properly.

haha ah yes you right. That I should have thought off.. :) back to testing..

comment:14 by luroh, 8 years ago

modeenf: This has been broken for a week now, effectively preventing much of VM testing. When do you plan to fix this?

comment:15 by tonestone57, 8 years ago

@modeenf, thanks for reverting in hrev41940. Please fix on your computer before committing. Test with all 3 VirtualBox adapters to confirm.

@Axel, Ok, thanks for that info. I checked the revision log and saw the FreeBSD driver imported and not changed. First by hugosantos, hrev21087 without changing in any following commits. 2nd, by Axel, hrev22991 but could not tell if import only or if changed because you appear to have done everything in one commit (I believe minor changes?) and 3rd by julun, hrev29246 with changes only to coding style in following commit. In all previous cases, the changes to the original BSD driver were zero to minor compared to what first seemed like Modeen's major driver rewrite though part of those changes could be from BSD driver update. Modeen also imported BSD driver and made changes to it in 1 commit making it impossible to follow.

@modeenf & others, it is best to import the original BSD driver into trunk first and then apply changes/fixes in following commit(s). That makes it better and easier for others to see what is going on. Refer to hrev29246 (import) & hrev29247 (changes) to see the proper and clean way of handling BSD drivers.

in reply to:  15 comment:16 by siarzhuk, 8 years ago

Replying to tonestone57:

@modeenf & others, it is best to import the original BSD driver into trunk first and then apply changes/fixes in following commit(s).

AFAIK the 3rd party stuff must be first imported into the vendor branch and only after this merged into trunk. Look into comments to hrev41815 for details. ;-)

in reply to:  15 comment:17 by axeld, 8 years ago

Replying to tonestone57:

@modeenf & others, it is best to import the original BSD driver into trunk first and then apply changes/fixes in following commit(s). That makes it better and easier for others to see what is going on. Refer to hrev29246 (import) & hrev29247 (changes) to see the proper and clean way of handling BSD drivers.

No, trunk should always work. As siarzhuk pointed out, you update the vendor branch first (there you can see what exactly has changed, and would then merge this diff to trunk -- fixing all what needs to be fixed. You can easily see what changes were necessary by comparing the vendor branch with trunk.

comment:18 by modeenf, 8 years ago

Yes sorry for all the problems..

One thing. What directory in vendor/freebsd should I update? I would go for renaming em and updating that one in current but the old freebds 7 code would still be in RELENG_7.

So this one can be closed for now?

comment:19 by luroh, 8 years ago

Resolution: fixed
Status: newclosed

Problem solved in hrev41940.

Note: See TracTickets for help on using tickets.