Page MenuHomeFreeBSD

Revamp ixgbe into seperate if_ix and if_ixv drivers
ClosedPublic

Authored by jfv on Jan 21 2015, 10:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 23 2017, 6:59 AM
Unknown Object (File)
Apr 14 2017, 9:13 PM
Unknown Object (File)
Apr 13 2017, 5:05 PM
Unknown Object (File)
Apr 12 2017, 6:56 PM
Unknown Object (File)
Apr 12 2017, 1:31 PM
Unknown Object (File)
Feb 22 2017, 8:41 PM
Unknown Object (File)
Jan 6 2017, 10:30 AM
Unknown Object (File)
May 5 2016, 5:13 PM
Subscribers

Details

Summary

This delta is very large, but its necessary. In preparation for full SRIOV support for this and the 40G
driver its necessary to seperate them.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

jfv retitled this revision from to Revamp ixgbe into seperate if_ix and if_ixv drivers.
jfv updated this object.
jfv edited the test plan for this revision. (Show Details)
jfv added reviewers: gnn, jhb, rstone, marcel.

We'll pop this into test and see what happens.

head/sys/conf/NOTES
2104

Do you want to update this comment to reflect 10/40gbe ethernet?

head/sys/conf/files
1768

This requires a kernel options change in sys/conf ?

This also requires a kernel config change in GENERIC?

head/sys/dev/ixgbe/if_ixv.c
74

Same as earlier, should this be 10/40GBE?

Can you provide more details on why you need to separate them (and why you need to rename them)? Note that a single kld can contain multiple drivers (like if_lem.c and if_em.c in if_em.ko).

I do think having the kld name match the interface is good (so the ifconfig auto-load magic works), but it's not clear to me why you can't just include the VF driver in the PF driver's module? (And renaming options in a kernel config is pretty disruptive, so there need to be good reasons.. People are probably still somewhat burned over emX devices changing to igbX at one point.)

In D1580#6, @jhb wrote:

Can you provide more details on why you need to separate them (and why you need to rename them)? Note that a single kld can contain multiple drivers (like if_lem.c and if_em.c in if_em.ko).

I do think having the kld name match the interface is good (so the ifconfig auto-load magic works), but it's not clear to me why you can't just include the VF driver in the PF driver's module? (And renaming options in a kernel config is pretty disruptive, so there need to be good reasons.. People are probably still somewhat burned over emX devices changing to igbX at one point.)

The big reason for seperating the VF is that once we have the ability to HOST, its essential that the VF driver
does not load.

You mean to avoid having the driver claim the VFs? That is going to be solved differently I think. Ryan's SRIOV changes already allow the driver to be forced to be "ppt" to support pass through, and the devctl tool I'm committing in the next day or so will allow the administrator to change this on the fly. I think those are the better routes to address this.

Hi Eric,

For those two patches below, could you point to me where did you address it?
https://reviews.freebsd.org/D823
https://reviews.freebsd.org/D811

All the rest looks good for me.

@araujo I added inline comments to where I made the changes.

In D1580#11, @araujo wrote:

Hi Eric,

For those two patches below, could you point to me where did you address it?
https://reviews.freebsd.org/D823
https://reviews.freebsd.org/D811

All the rest looks good for me.

head/sys/conf/NOTES
2104

ixl is the one that is for 40GbE products; though that one's comment might need updating because there are X710/10gig products out there.

head/sys/dev/ixgbe/if_ix.c
1490

@araujo -- 1000baseT should be correctly displayed now. D811

1588

@araujo -- This is where you can change the media type to 10GbaseT, or other media types.

2519

@araujo -- This is where 10GBaseT and other media types are added to the ifconfig -m list

araujo edited edge metadata.

Hi,

For those changes I mentioned before:
https://reviews.freebsd.org/D823
https://reviews.freebsd.org/D811

This version does the job.
Thanks Eric,

This revision is now accepted and ready to land.Feb 4 2015, 2:20 AM
In D1580#11, @araujo wrote:

Hi Eric,

For those two patches below, could you point to me where did you address it?
https://reviews.freebsd.org/D823
https://reviews.freebsd.org/D811

All the rest looks good for me.

Hi Eric,

I got approve for D823 and D811 from @jfvogel, I would like to commit it.
Do you have any objection?

Best Regards,

head/sys/dev/ixgbe/if_ix.c
1615

Would it might be 10G instead of 1GB_FULL?

1621

1G instead of 100?

Looks like this can be closed as it was committed?

jfv added inline comments.
head/sys/conf/NOTES
2104

What did you mean here Sean, this is not for 40G?

head/sys/conf/files
1768

Oh, you may be right about GENERIC, I'll add that.

head/sys/dev/ixgbe/if_ixv.c
74

Are you confusing this with ixl/ixlv, or did you somehow want the 4x10 skew to
be called 40G, I think that would just lead to confusion.