Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 11/19/14 22:34, Hans Petter Selasky wrote: On 11/19/14 21:46, K. Macy wrote: Hi Hans, It mostly looks fine, but it's a large change and there are some places in the patch where it isn't clear that the right thing is being done by looking at the patch alone. Please give us some time to review. No problem. Do you think you need more than a week? --HPS Hi, Do you need more time to review my patch? Any issues that should be fixed? --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
Hi, I need a little more time to review this. Sorry :( -a On 27 November 2014 at 09:10, Hans Petter Selasky h...@selasky.org wrote: On 11/19/14 22:34, Hans Petter Selasky wrote: On 11/19/14 21:46, K. Macy wrote: Hi Hans, It mostly looks fine, but it's a large change and there are some places in the patch where it isn't clear that the right thing is being done by looking at the patch alone. Please give us some time to review. No problem. Do you think you need more than a week? --HPS Hi, Do you need more time to review my patch? Any issues that should be fixed? --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 11/27/14 18:13, Adrian Chadd wrote: Hi, I need a little more time to review this. Sorry :( Hi, How much approximately? One day, half a week or more? Thank you for spending time on this! --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
Erk - SCTP folk - are you using the mbuf flowid field for something SCTP specific? -adrian On 27 November 2014 at 09:13, Adrian Chadd adr...@freebsd.org wrote: Hi, I need a little more time to review this. Sorry :( -a On 27 November 2014 at 09:10, Hans Petter Selasky h...@selasky.org wrote: On 11/19/14 22:34, Hans Petter Selasky wrote: On 11/19/14 21:46, K. Macy wrote: Hi Hans, It mostly looks fine, but it's a large change and there are some places in the patch where it isn't clear that the right thing is being done by looking at the patch alone. Please give us some time to review. No problem. Do you think you need more than a week? --HPS Hi, Do you need more time to review my patch? Any issues that should be fixed? --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 27 November 2014 at 09:18, Adrian Chadd adr...@freebsd.org wrote: Erk - SCTP folk - are you using the mbuf flowid field for something SCTP specific? erk - yes, you are. It seems we're going to run into what exactly should flowid be used for problems. Grr. -adrian ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 11/27/14 18:20, Adrian Chadd wrote: On 27 November 2014 at 09:18, Adrian Chadd adr...@freebsd.org wrote: Erk - SCTP folk - are you using the mbuf flowid field for something SCTP specific? erk - yes, you are. It seems we're going to run into what exactly should flowid be used for problems. Hi, If the flowid has special meaning inside the SCTP, please define an own rsstype for it. As far as I could see, it is only used to spread the traffic on the network adapter and on the CPU cores. --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 27 November 2014 at 09:25, Hans Petter Selasky h...@selasky.org wrote: On 11/27/14 18:20, Adrian Chadd wrote: On 27 November 2014 at 09:18, Adrian Chadd adr...@freebsd.org wrote: Erk - SCTP folk - are you using the mbuf flowid field for something SCTP specific? erk - yes, you are. It seems we're going to run into what exactly should flowid be used for problems. Hi, If the flowid has special meaning inside the SCTP, please define an own rsstype for it. As far as I could see, it is only used to spread the traffic on the network adapter and on the CPU cores. I'm more worried about at what layers we're going to be using the flowid values and that various pieces may stomp over other pieces. With the RSS stuff enabled, the IPv4 (and soon IPv6) input paths will re-hash an input frame if it doesn't have an RSS hash; it'll then overwrite whatever flowid value is in the mbuf. This is so no matter what the NIC hands us or what de-encapsulation hands us, we will always put that flow on into the right RSS bucket and a consistent CPU. This should mostly alleviate any out-of-order issues seen with internet-facing machines where things handle fragments and such. Then for the output side of things, it'll have to do a software RSS hash on frames that don't have an RSS hash. Right now I assume in the TCP path that the inp has an RSS flow setup in the inp and that the TCP timers and other assorted stuff uses the inp details. For the UDP output side of things I currently always re-calculate the RSS hash and stamp the mbuf with it before we send it out, again so it ends up on the same output RSS bucket and thus CPU / NIC queue. If the inp ends up with a different flowid (eg a hardware ring flowid) then: * it won't match up on the receive side, so the whole RSS lock contention avoidance thing can't happen; * there's a known mapping for RSS bucket - CPU IDs (since we're not doing RSS bucket - CPU ID reassignment yet) - but not for other flowid types to CPU IDs. In general I think the tidyup patch looks fine and I'll do some more RSS testing once it's committed. (But you did sneak in your new hash type in the diff :-) I think we then need to actually plan out how this stuff should hold together before we all rush into it or we'll end up with a mess of components that can't actually interact together. I don't want to have to explain to people that they can't use SCTP, RSS and hardware ring / flow assignment at the same time. It's just going to be painful in the long run. -adrian ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 11/27/14 18:33, Adrian Chadd wrote: On 27 November 2014 at 09:25, Hans Petter Selasky h...@selasky.org wrote: On 11/27/14 18:20, Adrian Chadd wrote: On 27 November 2014 at 09:18, Adrian Chadd adr...@freebsd.org wrote: Erk - SCTP folk - are you using the mbuf flowid field for something SCTP specific? erk - yes, you are. It seems we're going to run into what exactly should flowid be used for problems. Hi, If the flowid has special meaning inside the SCTP, please define an own rsstype for it. As far as I could see, it is only used to spread the traffic on the network adapter and on the CPU cores. I'm more worried about at what layers we're going to be using the flowid values and that various pieces may stomp over other pieces. With the RSS stuff enabled, the IPv4 (and soon IPv6) input paths will re-hash an input frame if it doesn't have an RSS hash; it'll then overwrite whatever flowid value is in the mbuf. This is so no matter what the NIC hands us or what de-encapsulation hands us, we will always put that flow on into the right RSS bucket and a consistent CPU. This should mostly alleviate any out-of-order issues seen with internet-facing machines where things handle fragments and such. Hi, I'm not changing anything in the receive direction regarding the flowid. It should be exactly the same as before, except M_HASHTYPE_NONE which now indicates that flowid is not set. Then for the output side of things, it'll have to do a software RSS hash on frames that don't have an RSS hash. Right now I assume in the TCP path that the inp has an RSS flow setup in the inp and that the TCP timers and other assorted stuff uses the inp details. For the UDP output side of things I currently always re-calculate the RSS hash and stamp the mbuf with it before we send it out, again so it ends up on the same output RSS bucket and thus CPU / NIC queue. If the inp ends up with a different flowid (eg a hardware ring flowid) then: * it won't match up on the receive side, so the whole RSS lock contention avoidance thing can't happen; * there's a known mapping for RSS bucket - CPU IDs (since we're not doing RSS bucket - CPU ID reassignment yet) - but not for other flowid types to CPU IDs. Not necessarily. Would could make a standard, that the lower X-bits of the flowid, always indicate RX/TX ring pair and the CPU core, and then the upper 32-X bits are free to use for other purposes. In general I think the tidyup patch looks fine and I'll do some more RSS testing once it's committed. (But you did sneak in your new hash type in the diff :-) I'll put the new hash type in a separate patch. It doesn't belong there - you're right. I think we then need to actually plan out how this stuff should hold together before we all rush into it or we'll end up with a mess of components that can't actually interact together. I don't want to have to explain to people that they can't use SCTP, RSS and hardware ring / flow assignment at the same time. It's just going to be painful in the long run. Do you have code not committed which plan to use the flowid in new areas of the FreeBSD kernel? I would like to have a list of usages for the flowid field? --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
Hm, how are we going to have the RSS stuff work at the same time as the hardware flow steering stuff you're prototyping? -adrian On 19 November 2014 11:02, Hans Petter Selasky h...@selasky.org wrote: Hi, The M_FLOWID flag is marked as deprecated in the FreeBSD kernel code and the patch below completely removes it. I suggest we will now be using the m_pkthdr.rsstype also known as M_HASHTYPE to decide if the flowid value is valid or not. When the rsstype is set to M_HASHTYPE_NONE the m_pkthdr.flowid field is not valid. Else this field contains valid data for both TX and RX direction. Background: === The network drivers today use the rsstype field only when receiving traffic. After my patch it is also used when sending traffic, and probably we should rename it. The reason for using the rsstype field for transmit, is to avoid introducing another field in the MBUF's packet header in order to steer outgoing traffic into special multiple purpose hardware FIFOs. This new feature should coexist with the existing flowid mechanism, and this is achieved by introducing a new hash type which I've named M_HASHTYPE_HWRING in my patch. This type can be selected by upper layers when generating traffic for lower layers, to indicate that the traffic is of a special kind and should have special treatment by the hardware, like rate-limiting. Hardware which doesn't support M_HASHTYPE_HWRING will send out the packets like usual. Patch is available from here: = http://home.selasky.org:8192/m_flowid_removal.diff Comments are appreciated! --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 11/19/14 20:23, Adrian Chadd wrote: Hm, how are we going to have the RSS stuff work at the same time as the hardware flow steering stuff you're prototyping? Hi Adrain, RSS is only the receive side and its functionality is not touched. I'm just re-using the RSS fields for the transmit side, where the rsstype is currently not used. Do you see anything broken in my patch? --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 19 Nov 2014, at 20:02, Hans Petter Selasky h...@selasky.org wrote: Hi, The M_FLOWID flag is marked as deprecated in the FreeBSD kernel code and the patch below completely removes it. I suggest we will now be using the m_pkthdr.rsstype also known as M_HASHTYPE to decide if the flowid value is valid or not. When the rsstype is set to M_HASHTYPE_NONE the m_pkthdr.flowid field is not valid. Else this field contains valid data for both TX and RX direction. Background: === The network drivers today use the rsstype field only when receiving traffic. After my patch it is also used when sending traffic, and probably we should rename it. The reason for using the rsstype field for transmit, is to avoid introducing another field in the MBUF's packet header in order to steer outgoing traffic into special multiple purpose hardware FIFOs. This new feature should coexist with the existing flowid mechanism, and this is achieved by introducing a new hash type which I've named M_HASHTYPE_HWRING in my patch. This type can be selected by upper layers when generating traffic for lower layers, to indicate that the traffic is of a special kind and should have special treatment by the hardware, like rate-limiting. Hardware which doesn't support M_HASHTYPE_HWRING will send out the packets like usual. Patch is available from here: = http://home.selasky.org:8192/m_flowid_removal.diff Comments are appreciated! Before finally committing this, drop me a note. All the SCTP changes need to be ported upstream. Depending on the time and if it is OK for you, I would try to integrate this upstream and push it down to FreeBSD. Then you can commit the rest. This is simpler for me than reintegrating your changes upstream... Best regards Michael --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
The RSS hash is also used for: * TCP timers, * UDP transmit, and * the transmit path in RSS aware drivers (igb / ixgbe) -adrian On 19 November 2014 11:25, Hans Petter Selasky h...@selasky.org wrote: On 11/19/14 20:23, Adrian Chadd wrote: Hm, how are we going to have the RSS stuff work at the same time as the hardware flow steering stuff you're prototyping? Hi Adrain, RSS is only the receive side and its functionality is not touched. I'm just re-using the RSS fields for the transmit side, where the rsstype is currently not used. Do you see anything broken in my patch? --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 11/19/14 20:32, Adrian Chadd wrote: The RSS hash is also used for: * TCP timers, * UDP transmit, and * the transmit path in RSS aware drivers (igb / ixgbe) I know, and the RSS flowid values are still preserved as before in the receive path. It is just about how you tell the upper/lower layers that there is something in the m_pkthdr.flowid which they can use to accelerate traffic. --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
Hi Hans, It mostly looks fine, but it's a large change and there are some places in the patch where it isn't clear that the right thing is being done by looking at the patch alone. Please give us some time to review. Thanks. -K On Wed, Nov 19, 2014 at 11:02 AM, Hans Petter Selasky h...@selasky.org wrote: Hi, The M_FLOWID flag is marked as deprecated in the FreeBSD kernel code and the patch below completely removes it. I suggest we will now be using the m_pkthdr.rsstype also known as M_HASHTYPE to decide if the flowid value is valid or not. When the rsstype is set to M_HASHTYPE_NONE the m_pkthdr.flowid field is not valid. Else this field contains valid data for both TX and RX direction. Background: === The network drivers today use the rsstype field only when receiving traffic. After my patch it is also used when sending traffic, and probably we should rename it. The reason for using the rsstype field for transmit, is to avoid introducing another field in the MBUF's packet header in order to steer outgoing traffic into special multiple purpose hardware FIFOs. This new feature should coexist with the existing flowid mechanism, and this is achieved by introducing a new hash type which I've named M_HASHTYPE_HWRING in my patch. This type can be selected by upper layers when generating traffic for lower layers, to indicate that the traffic is of a special kind and should have special treatment by the hardware, like rate-limiting. Hardware which doesn't support M_HASHTYPE_HWRING will send out the packets like usual. Patch is available from here: = http://home.selasky.org:8192/m_flowid_removal.diff Comments are appreciated! --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On 11/19/14 21:46, K. Macy wrote: Hi Hans, It mostly looks fine, but it's a large change and there are some places in the patch where it isn't clear that the right thing is being done by looking at the patch alone. Please give us some time to review. No problem. Do you think you need more than a week? --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] Removal of M_FLOWID flag from m_flags [WAS: Add support for hardware transmit rate limiting queues]
On Wed, Nov 19, 2014 at 1:34 PM, Hans Petter Selasky h...@selasky.org wrote: On 11/19/14 21:46, K. Macy wrote: Hi Hans, It mostly looks fine, but it's a large change and there are some places in the patch where it isn't clear that the right thing is being done by looking at the patch alone. Please give us some time to review. No problem. Do you think you need more than a week? (Didn't CC the others last time) I probably won't. But I speak only for myself. However, I don't think it's fair to ask you to wait more than two. I've been on the other end of that too many times myself. -K ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
