[PATCH] iscsid: Add support for net_prio cgroups
Mark Rustad
mechanism, since it can handle changes dynamically, as opposed to the
current method used with DCB. Continue to use the current DCB method
when net_prio cgroups are not present.
Signed-off-by: Mark Rustad <mark.d...@intel.com>
---
usr/dcb_app.c | 36 +++++++++++++++++++++++++++++++++++-
1 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/usr/dcb_app.c b/usr/dcb_app.c
index c31e2b8..0c509cc 100644
--- a/usr/dcb_app.c
+++ b/usr/dcb_app.c
@@ -1,7 +1,7 @@
/*******************************************************************************
DCB application support
- Copyright(c) 2007-2011 Intel Corporation.
+ Copyright(c) 2007-2012 Intel Corporation.
This program is free software; you can redistribute it and/or modify it
under the terms and conditions of the GNU General Public License,
@@ -25,6 +25,8 @@
*******************************************************************************/
#include <unistd.h>
+#include <stdbool.h>
+#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
@@ -57,6 +59,9 @@
/* Maximum size of response requested or message sent */
#define MAX_MSG_SIZE 1024
+#define CGROUPS_PATH "/proc/cgroups"
+#define NET_PRIO_CG "net_prio"
+
static struct nlmsghdr *start_dcbmsg(__u16 msg_type, __u8 arg)
{
struct nlmsghdr *nlh;
@@ -337,6 +342,32 @@ static int get_link_ifname(const char *ifname, char *link_ifname)
return 0;
}
+static bool net_prio_cgroup(void)
+{
+ FILE *cgs;
+ char line[80];
+ static bool inited;
+ static bool result;
+
+ if (inited)
+ return result;
+ cgs = fopen(CGROUPS_PATH, "r");
+ if (!cgs)
+ return false;
+
+ result = false;
+ while (fgets(line, sizeof(line), cgs)) {
+ if (strncmp(NET_PRIO_CG "\t", line, sizeof(NET_PRIO_CG)))
+ continue;
+ result = true;
+ break;
+ }
+
+ inited = true;
+ fclose(cgs);
+ return result;
+}
+
static int get_app_pri(const char *iface, __u8 req_idtype, __u16 req_id,
__u8 ieee_mask)
{
@@ -345,6 +376,9 @@ static int get_app_pri(const char *iface, __u8 req_idtype, __u16 req_id,
int nl_sd;
char ifname[IFNAMSIZ];
+ if (net_prio_cgroup())
+ return 0;
+
if (get_link_ifname(iface, ifname))
return 0;
Rustad, Mark D
This patch assumes that the cgroup will at least be present before iscsid gets started if it is going to be used to control network priorities. If that proves not to be true some changes will be necessary, as you can see this implementation only checks for the net_prio cgroup one time (to avoid the overhead of the added check for each session startup). Some coordination with the net_prio cgroup implementation may be necessary here.
As this patch has not yet been tested, please send any suggestions, complaints or observations to me. Thanks.
--
Mark Rustad, LAN Access Division, Intel Corporation
Shyam...@dell.com
> -----Original Message-----
> From: open-...@googlegroups.com [mailto:open-...@googlegroups.com]
> On Behalf Of Rustad, Mark D
> Sent: Friday, February 24, 2012 8:30 PM
> To: <open-...@googlegroups.com>
> Subject: Re: [PATCH] iscsid: Add support for net_prio cgroups
>
> I have sent this patch to improve the behavior of iscsid when the new
> net_prio cgroup support is in place to manage network application
> priorities. This patch detects the presence of the net_prio cgroup and
> then blocks all use of the current DCB support in such a case. This is
> because a priority set by an application will prevent the net_prio
> cgroup from controlling the application. One reason to prefer having
> the net_prio cgroup control the priority is that it can dynamically
> respond to changes in an application's priority as communicated through
> DCB, which the current DCB support in iscsid cannot do.
>
> This patch assumes that the cgroup will at least be present before
> iscsid gets started if it is going to be used to control network
> priorities.
Missing init script implementation.. ?
If that proves not to be true some changes will be
> necessary, as you can see this implementation only checks for the
> net_prio cgroup one time (to avoid the overhead of the added check for
> each session startup). Some coordination with the net_prio cgroup
> implementation may be necessary here.
>
> As this patch has not yet been tested, please send any suggestions,
> complaints or observations to me. Thanks.
>
> --
> Mark Rustad, LAN Access Division, Intel Corporation
>
> --
> You received this message because you are subscribed to the Google
> Groups "open-iscsi" group.
> To post to this group, send email to open-...@googlegroups.com.
> To unsubscribe from this group, send email to open-
> iscsi+un...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/open-iscsi?hl=en.
Rustad, Mark D
<Shyam...@Dell.com> wrote:
> Missing init script implementation.. ?
There is no init script directly related to this patch to open-iscsi, or do you have something else in mind? If you are referring to the net_prio cgroup implementation, I'm not fully in the loop on that (or else I might know more about how things are sequenced at startup).
What do you have in mind for init scripts?
Mike Christie
> When net_prio cgroups are in use, let the priority be controlled by that
> mechanism, since it can handle changes dynamically, as opposed to the
> current method used with DCB. Continue to use the current DCB method
> when net_prio cgroups are not present.
When it is changed, is there some event from the kernel/userspace that
we can listen for or will it cause the current connection to be come
disconnected and then on the reconnect we recheck and that is how we get
the update?
Code wise patch looks ok
Rustad, Mark D
On Feb 27, 2012, at 1:24 PM, Mike Christie wrote:
> When it is changed, is there some event from the kernel/userspace that
> we can listen for or will it cause the current connection to be come
> disconnected and then on the reconnect we recheck and that is how we get
> the update?
It is simpler and more direct than that. On packet egress, if the skb has no priority and came from a socket, the socket is checked to see if it is in a cgroup. If it is, and a priority has been configured for that cgroup and interface, then the priority is applied to the skb at that point. So there is no session disruption at all.
A separate daemon takes DCB app priority changes as they happen and sets them up for the necessary cgroups. So there is an event from the kernel, but this new daemon does all of the processing for it. That is what drives the changes.
Since if an skb already has a priority the net_prio cgroup code will not be executed, this patch simply prevents iscsid from setting a priority when the net_prio cgroup exists.
> Code wise patch looks ok
Yeah, ultimately it would be nice to simply remove all of the DCB-related code from iscsid, since with the net_prio cgroups, it can all happen in a better way without any of it. The only reason not to do that now, is just to avoid any disruption to those using it as it is today. I knew that the current DCB implementation that I submitted last year would not stand - and I was working on a better, simpler way to do it - but I was not expecting we could get to a place where no app change was needed at all. That was a very pleasant surprise. At least the current implementation enabled some exploration and evaluation of iSCSI with DCB. I think you are in a better position than I to judge when the DCB code can simply be removed.
Anish Bhatt
Has there been any progress in figuring out how DCB would co-exist with net prio cgroups since you removed DCB support ?
-Anish
Mike Christie
> Mike,
> Has there been any progress in figuring out how DCB would
> co-exist with net prio cgroups since you removed DCB support ?
> -Anish
Are you guys using DCB with iSCSI for software iscsi or with cxgb*i?
> You received this message because you are subscribed to the Google
> Groups "open-iscsi" group.
> https://groups.google.com/d/msg/open-iscsi/-/FmX53RYghW8J.
> To unsubscribe from this group, send email to
Anish Bhatt
had DCB support for the software initiator. I'm guessing with cgdcbxd
the task of looking up cgroup and assigning priority to the traffic is
left to the individual transports now ?
-Anish
One socket to bind them all.
> -----Original Message-----
> From: Mike Christie [mailto:mich...@cs.wisc.edu]
> Sent: Monday, June 11, 2012 3:40 PM
> To: open-...@googlegroups.com
> Cc: Anish Bhatt
> Subject: Re: [PATCH] iscsid: Add support for net_prio cgroups
>
Mike Christie
> I was looking at support for cxgb*i, since open-iscsi previously only
> had DCB support for the software initiator. I'm guessing with cgdcbxd
> the task of looking up cgroup and assigning priority to the traffic is
> left to the individual transports now ?
offloaded iscsi connection and it just works?
It is only iscsi_tcp and cxgb*i that need this. The other offload
drivers are doing everything in fw/hw.
Anish Bhatt
was just checking if there was any plan to make the transports interact
with cgroups in a uniform way, as previously DCB was left upto
individual implementations. Guess not.
One socket to bind them all.
> -----Original Message-----
> From: Mike Christie [mailto:mich...@cs.wisc.edu]
> Sent: Monday, June 11, 2012 4:17 PM
> To: open-...@googlegroups.com
> Cc: Anish Bhatt
> Subject: Re: [PATCH] iscsid: Add support for net_prio cgroups
>