| Message ID | 20170913050122.13731-1-nakayamakenjiro@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 9/13/17 12:01 AM, Kenjiro Nakayama wrote: > Currently netlink command reply support option > (TCMU_ATTR_SUPP_KERN_CMD_REPLY) can be enabled only on module > scope. Because of that, once an application enables the netlink > command reply support, all applications using target_core_user.ko > would be expected to support the netlink reply. To make matters worse, > users will not be able to add a device via configfs manually. > > To fix these issues, this patch adds an option to make netlink command > reply disabled on each device through configfs. Original > TCMU_ATTR_SUPP_KERN_CMD_REPLY is still enabled on module scope to keep > backward-compatibility and used by default, however once users set > nl_reply_supported=<NAGATIVE_VALUE> via configfs for a particular > device, the device disables the netlink command reply support. > > Signed-off-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com> > --- > drivers/target/target_core_user.c | 59 ++++++++++++++++++++++++++++++++++++++- > 1 file changed, 58 insertions(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c > index 942d094269fb..709c27ed4206 100644 > --- a/drivers/target/target_core_user.c > +++ b/drivers/target/target_core_user.c > @@ -150,6 +150,8 @@ struct tcmu_dev { > wait_queue_head_t nl_cmd_wq; > > char dev_config[TCMU_CONFIG_LEN]; > + > + int nl_reply_supported; > }; > > #define TCMU_DEV(_se_dev) container_of(_se_dev, struct tcmu_dev, se_dev) > @@ -1306,6 +1308,10 @@ static void tcmu_init_genl_cmd_reply(struct tcmu_dev *udev, int cmd) > > if (!tcmu_kern_cmd_reply_supported) > return; > + > + if (udev->nl_reply_supported <= 0) > + return; > + > relock: > spin_lock(&udev->nl_cmd_lock); > > @@ -1332,6 +1338,9 @@ static int tcmu_wait_genl_cmd_reply(struct tcmu_dev *udev) > if (!tcmu_kern_cmd_reply_supported) > return 0; > > + if (udev->nl_reply_supported <= 0) > + return 0; > + > pr_debug("sleeping for nl reply\n"); > wait_for_completion(&nl_cmd->complete); > > @@ -1506,6 +1515,12 @@ static int tcmu_configure_device(struct se_device *dev) > dev->dev_attrib.emulate_write_cache = 0; > dev->dev_attrib.hw_queue_depth = 128; > > + /* If user didn't explicitly disable netlink reply support, use > + * module scope setting. > + */ > + if (udev->nl_reply_supported >= 0) > + udev->nl_reply_supported = tcmu_kern_cmd_reply_supported; > + > /* > * Get a ref incase userspace does a close on the uio device before > * LIO has initiated tcmu_free_device. > @@ -1610,7 +1625,7 @@ static void tcmu_destroy_device(struct se_device *dev) > > enum { > Opt_dev_config, Opt_dev_size, Opt_hw_block_size, Opt_hw_max_sectors, > - Opt_err, > + Opt_nl_reply_supported, Opt_err, > }; > > static match_table_t tokens = { > @@ -1618,6 +1633,7 @@ static match_table_t tokens = { > {Opt_dev_size, "dev_size=%u"}, > {Opt_hw_block_size, "hw_block_size=%u"}, > {Opt_hw_max_sectors, "hw_max_sectors=%u"}, > + {Opt_nl_reply_supported, "nl_reply_supported=%d"}, > {Opt_err, NULL} > }; > > @@ -1692,6 +1708,18 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev, > ret = tcmu_set_dev_attrib(&args[0], > &(dev->dev_attrib.hw_max_sectors)); > break; > + case Opt_nl_reply_supported: > + arg_p = match_strdup(&args[0]); > + if (!arg_p) { > + ret = -ENOMEM; > + break; > + } > + ret = kstrtol(arg_p, 0, > + (long int *) &udev->nl_reply_supported); > + kfree(arg_p); > + if (ret < 0) > + pr_err("kstrtoul() failed for nl_reply_supported=\n"); > + break; > default: > break; > } > @@ -1842,6 +1870,34 @@ static ssize_t tcmu_dev_size_store(struct config_item *item, const char *page, > } > CONFIGFS_ATTR(tcmu_, dev_size); > > +static ssize_t tcmu_nl_reply_supported_show(struct config_item *item, > + char *page) > +{ > + struct se_dev_attrib *da = container_of(to_config_group(item), > + struct se_dev_attrib, da_group); > + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); > + > + return snprintf(page, PAGE_SIZE, "%d\n", udev->nl_reply_supported); > +} > + > +static ssize_t tcmu_nl_reply_supported_store(struct config_item *item, > + const char *page, size_t count) > +{ > + struct se_dev_attrib *da = container_of(to_config_group(item), > + struct se_dev_attrib, da_group); > + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); > + s8 val; > + int ret; > + > + ret = kstrtos8(page, 0, &val); > + if (ret < 0) > + return ret; > + > + udev->nl_reply_supported = val; > + return count; > +} > +CONFIGFS_ATTR(tcmu_, nl_reply_supported); > + > static ssize_t tcmu_emulate_write_cache_show(struct config_item *item, > char *page) > { > @@ -1884,6 +1940,7 @@ static struct configfs_attribute *tcmu_attrib_attrs[] = { > &tcmu_attr_dev_config, > &tcmu_attr_dev_size, > &tcmu_attr_emulate_write_cache, > + &tcmu_attr_nl_reply_supported, > NULL, > }; > The problem with this patch is that for tcmu-runner/targetcli-fb users the code will depend on the genl_info structure that contains whether or not a netlink reply is supported. You will need to fix that path where if you were to update the configfs to disable netlink reply and if a user already has stuff setup then things will get messy. Userspace will think netlink reply is supported, but kernel doesn't expect one. -Bryant -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/13/2017 12:01 AM, Kenjiro Nakayama wrote: > Currently netlink command reply support option > (TCMU_ATTR_SUPP_KERN_CMD_REPLY) can be enabled only on module > scope. Because of that, once an application enables the netlink > command reply support, all applications using target_core_user.ko I am ok with the idea for the patch, but what type of setup do you have where there are multiple applications using the interface and some support it and some do not? Is it because tcmu-runner is starting by default due to something like a distro systemd unit file and then you also have your app running too? Also, if you do not implement sync netlink support, how does your daemon tell the kernel if the device failed to setup in the daemon? For deletion how did you work around the uio crashes and leaks? > would be expected to support the netlink reply. To make matters worse, > users will not be able to add a device via configfs manually. > What is the specific issue with manually setting it up through configfs? rtslib/tagretcli use the same configfs operations and does not know what tcmu and daemons like tcmu-runner support. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/13/2017 10:10 AM, Bryant G. Ly wrote: > > On 9/13/17 12:01 AM, Kenjiro Nakayama wrote: > >> Currently netlink command reply support option >> (TCMU_ATTR_SUPP_KERN_CMD_REPLY) can be enabled only on module >> scope. Because of that, once an application enables the netlink >> command reply support, all applications using target_core_user.ko >> would be expected to support the netlink reply. To make matters worse, >> users will not be able to add a device via configfs manually. >> >> To fix these issues, this patch adds an option to make netlink command >> reply disabled on each device through configfs. Original >> TCMU_ATTR_SUPP_KERN_CMD_REPLY is still enabled on module scope to keep >> backward-compatibility and used by default, however once users set >> nl_reply_supported=<NAGATIVE_VALUE> via configfs for a particular >> device, the device disables the netlink command reply support. >> >> Signed-off-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com> >> --- >> drivers/target/target_core_user.c | 59 >> ++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 58 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/target/target_core_user.c >> b/drivers/target/target_core_user.c >> index 942d094269fb..709c27ed4206 100644 >> --- a/drivers/target/target_core_user.c >> +++ b/drivers/target/target_core_user.c >> @@ -150,6 +150,8 @@ struct tcmu_dev { >> wait_queue_head_t nl_cmd_wq; >> >> char dev_config[TCMU_CONFIG_LEN]; >> + >> + int nl_reply_supported; >> }; >> >> #define TCMU_DEV(_se_dev) container_of(_se_dev, struct tcmu_dev, >> se_dev) >> @@ -1306,6 +1308,10 @@ static void tcmu_init_genl_cmd_reply(struct >> tcmu_dev *udev, int cmd) >> >> if (!tcmu_kern_cmd_reply_supported) >> return; >> + >> + if (udev->nl_reply_supported <= 0) >> + return; >> + >> relock: >> spin_lock(&udev->nl_cmd_lock); >> >> @@ -1332,6 +1338,9 @@ static int tcmu_wait_genl_cmd_reply(struct >> tcmu_dev *udev) >> if (!tcmu_kern_cmd_reply_supported) >> return 0; >> >> + if (udev->nl_reply_supported <= 0) >> + return 0; >> + >> pr_debug("sleeping for nl reply\n"); >> wait_for_completion(&nl_cmd->complete); >> >> @@ -1506,6 +1515,12 @@ static int tcmu_configure_device(struct >> se_device *dev) >> dev->dev_attrib.emulate_write_cache = 0; >> dev->dev_attrib.hw_queue_depth = 128; >> >> + /* If user didn't explicitly disable netlink reply support, use >> + * module scope setting. >> + */ >> + if (udev->nl_reply_supported >= 0) >> + udev->nl_reply_supported = tcmu_kern_cmd_reply_supported; >> + >> /* >> * Get a ref incase userspace does a close on the uio device before >> * LIO has initiated tcmu_free_device. >> @@ -1610,7 +1625,7 @@ static void tcmu_destroy_device(struct se_device >> *dev) >> >> enum { >> Opt_dev_config, Opt_dev_size, Opt_hw_block_size, >> Opt_hw_max_sectors, >> - Opt_err, >> + Opt_nl_reply_supported, Opt_err, >> }; >> >> static match_table_t tokens = { >> @@ -1618,6 +1633,7 @@ static match_table_t tokens = { >> {Opt_dev_size, "dev_size=%u"}, >> {Opt_hw_block_size, "hw_block_size=%u"}, >> {Opt_hw_max_sectors, "hw_max_sectors=%u"}, >> + {Opt_nl_reply_supported, "nl_reply_supported=%d"}, >> {Opt_err, NULL} >> }; >> >> @@ -1692,6 +1708,18 @@ static ssize_t >> tcmu_set_configfs_dev_params(struct se_device *dev, >> ret = tcmu_set_dev_attrib(&args[0], >> &(dev->dev_attrib.hw_max_sectors)); >> break; >> + case Opt_nl_reply_supported: >> + arg_p = match_strdup(&args[0]); >> + if (!arg_p) { >> + ret = -ENOMEM; >> + break; >> + } >> + ret = kstrtol(arg_p, 0, >> + (long int *) &udev->nl_reply_supported); >> + kfree(arg_p); >> + if (ret < 0) >> + pr_err("kstrtoul() failed for nl_reply_supported=\n"); >> + break; >> default: >> break; >> } >> @@ -1842,6 +1870,34 @@ static ssize_t tcmu_dev_size_store(struct >> config_item *item, const char *page, >> } >> CONFIGFS_ATTR(tcmu_, dev_size); >> >> +static ssize_t tcmu_nl_reply_supported_show(struct config_item *item, >> + char *page) >> +{ >> + struct se_dev_attrib *da = container_of(to_config_group(item), >> + struct se_dev_attrib, da_group); >> + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); >> + >> + return snprintf(page, PAGE_SIZE, "%d\n", udev->nl_reply_supported); >> +} >> + >> +static ssize_t tcmu_nl_reply_supported_store(struct config_item *item, >> + const char *page, size_t count) >> +{ >> + struct se_dev_attrib *da = container_of(to_config_group(item), >> + struct se_dev_attrib, da_group); >> + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); >> + s8 val; >> + int ret; >> + >> + ret = kstrtos8(page, 0, &val); >> + if (ret < 0) >> + return ret; >> + >> + udev->nl_reply_supported = val; >> + return count; >> +} >> +CONFIGFS_ATTR(tcmu_, nl_reply_supported); >> + >> static ssize_t tcmu_emulate_write_cache_show(struct config_item *item, >> char *page) >> { >> @@ -1884,6 +1940,7 @@ static struct configfs_attribute >> *tcmu_attrib_attrs[] = { >> &tcmu_attr_dev_config, >> &tcmu_attr_dev_size, >> &tcmu_attr_emulate_write_cache, >> + &tcmu_attr_nl_reply_supported, >> NULL, >> }; >> > > The problem with this patch is that for tcmu-runner/targetcli-fb users > the code will depend > on the genl_info structure that contains whether or not a netlink reply > is supported. You will > need to fix that path where if you were to update the configfs to > disable netlink reply and if a user > already has stuff setup then things will get messy. Userspace will think > netlink reply is supported, > but kernel doesn't expect one. > I don't think this will be a problem, because the patch has backwards compat support and the app that does not support nl replies will only set the nl_reply_support = -1 for the devices it is managing. If userspace were to mess up and set nl_reply_suppport = -1 for a device it did not manage and so a reply is sent when the kernel did not expect one, the kernel should just spit out an error due to not being able to find a device or cmd mismatch, or we might do a complete() with no waiters, or for device removal we might hit the uio crash again. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(re-sending this message as mail server returned error) > I am ok with the idea for the patch, but what type of setup do you have > where there are multiple applications using the interface and some > support it and some do not? Is it because tcmu-runner is starting by > default due to something like a distro systemd unit file and then you > also have your app running too? Though I am not going to run multiple applications using the support/non-support interface atm, I have to note to use the non-support app as "Please do not run tcmu-runner (or any app that enables netlink reply support) before/during running this app. If you ran it, you have to restart the host and never run it again". It sounds like a little bit silly note... > Also, if you do not implement sync netlink support, how does your daemon > tell the kernel if the device failed to setup in the daemon? For > deletion how did you work around the uio crashes and leaks? Yes, I admit that the application should implement the netlink sync support, and I know the issue when I am using the device without netlink sync. However, I believe that it would be good if there is an option to (force) disable the netlink sync for the workaround. It means that not only for the non-supported app, but also for the trouble shooting. (As I sent timeout patch to add timeout, it sometimes causes an issue like hang due to the netlink sync failure.) > > would be expected to support the netlink reply. To make matters worse, > > users will not be able to add a device via configfs manually. > > > > What is the specific issue with manually setting it up through configfs? > rtslib/tagretcli use the same configfs operations and does not know what > tcmu and daemons like tcmu-runner support. I meant the following steps: 1. Run tcmu-runner (to enable netlink reply support). // You can stop tcmu-runner process as it keeps the netlink support enabled // # ./tcmu-runner 2. Enable the new device // This will hang echo command forever // mkdir -p /sys/kernel/config/target/core/user_20/test echo "1" >> /sys/kernel/config/target/core/user_20/test/enable My application is using same steps inside the code. Regards, Kenjiro On Thu, Sep 14, 2017 at 12:58 AM, Mike Christie <mchristi@redhat.com> wrote: > On 09/13/2017 12:01 AM, Kenjiro Nakayama wrote: >> Currently netlink command reply support option >> (TCMU_ATTR_SUPP_KERN_CMD_REPLY) can be enabled only on module >> scope. Because of that, once an application enables the netlink >> command reply support, all applications using target_core_user.ko > > I am ok with the idea for the patch, but what type of setup do you have > where there are multiple applications using the interface and some > support it and some do not? Is it because tcmu-runner is starting by > default due to something like a distro systemd unit file and then you > also have your app running too? > > Also, if you do not implement sync netlink support, how does your daemon > tell the kernel if the device failed to setup in the daemon? For > deletion how did you work around the uio crashes and leaks? > >> would be expected to support the netlink reply. To make matters worse, >> users will not be able to add a device via configfs manually. >> > > What is the specific issue with manually setting it up through configfs? > rtslib/tagretcli use the same configfs operations and does not know what > tcmu and daemons like tcmu-runner support.
On 09/13/2017 01:28 PM, Nakayama Kenjiro wrote: >> I am ok with the idea for the patch, but what type of setup do you have >> where there are multiple applications using the interface and some >> support it and some do not? Is it because tcmu-runner is starting by >> default due to something like a distro systemd unit file and then you >> also have your app running too? > > Though I am not going to run multiple applications using the > support/non-support interface atm, I have to note to use the non-support > app as "Please do not run tcmu-runner (or any app that enables > netlink reply support) before/during running this app. If you ran it, you > have to restart the host and never run it again". It sounds like a > little bit silly note... Yes. Agree. > >> Also, if you do not implement sync netlink support, how does your daemon >> tell the kernel if the device failed to setup in the daemon? For >> deletion how did you work around the uio crashes and leaks? > > Yes, I admit that the application should implement the netlink sync > support, Actually maybe I would not add support for it at all if I were you. Some other questions/comments, because how the daemons like tcmu-runner do setup might be a result of using targetcli/rtslib, and if you are not going to use that then you can simplfy things. The current normal way to create a device is: 1. Start app which listens on nl. 2. targetcli/rtslib run configfs commands to add and enable tcmu device. 3. tcmu sends nl event to app which sets up its objects for the device and returns success/failure. This approach works well for the targetcli/rtslib device management flow, but has those extra steps. If you are not going to use targetcli/rtslib then you do not need netlink and to go through those extra steps. It sounds like you want to do: 1. app runs configfs commands to add and enable tcmu device. 2. app knows tcmu device name and other info so it can just setup its objects on successful writing to the enable file. 3. It sounds like you do not need the nl events at all at this point right. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> If you are not going to use targetcli/rtslib then you do not need > netlink and to go through those extra steps. > > It sounds like you want to do: > > 1. app runs configfs commands to add and enable tcmu device. > 2. app knows tcmu device name and other info so it can just setup its > objects on successful writing to the enable file. > 3. It sounds like you do not need the nl events at all at this point > right. Yes, that's correct I don't need any nl events actually. But my point is that when netlink reply support is enabled on the host by other app, writing to the enable file ($TARGET/user_N/device/enable) hangs because of waiting for the netlink sync in kernel. That's why I thought I should implement it. Currently there are no way to disable the netlink sync once enabled it and even no way to check if it is enable/disabled on the host. You don't think that it is necessary? At least, I would like to confirm the netlink sync was disabled before running a non-support netlink app. Regards, Kenjiro On Thu, Sep 14, 2017 at 3:43 AM, Mike Christie <mchristi@redhat.com> wrote: > On 09/13/2017 01:28 PM, Nakayama Kenjiro wrote: >>> I am ok with the idea for the patch, but what type of setup do you have >>> where there are multiple applications using the interface and some >>> support it and some do not? Is it because tcmu-runner is starting by >>> default due to something like a distro systemd unit file and then you >>> also have your app running too? >> >> Though I am not going to run multiple applications using the >> support/non-support interface atm, I have to note to use the non-support >> app as "Please do not run tcmu-runner (or any app that enables >> netlink reply support) before/during running this app. If you ran it, you >> have to restart the host and never run it again". It sounds like a >> little bit silly note... > > Yes. Agree. > >> >>> Also, if you do not implement sync netlink support, how does your daemon >>> tell the kernel if the device failed to setup in the daemon? For >>> deletion how did you work around the uio crashes and leaks? >> >> Yes, I admit that the application should implement the netlink sync >> support, > > Actually maybe I would not add support for it at all if I were you. > > Some other questions/comments, because how the daemons like tcmu-runner > do setup might be a result of using targetcli/rtslib, and if you are not > going to use that then you can simplfy things. > > The current normal way to create a device is: > > 1. Start app which listens on nl. > 2. targetcli/rtslib run configfs commands to add and enable tcmu device. > 3. tcmu sends nl event to app which sets up its objects for the device > and returns success/failure. > > This approach works well for the targetcli/rtslib device management > flow, but has those extra steps. > > If you are not going to use targetcli/rtslib then you do not need > netlink and to go through those extra steps. > > It sounds like you want to do: > > 1. app runs configfs commands to add and enable tcmu device. > 2. app knows tcmu device name and other info so it can just setup its > objects on successful writing to the enable file. > 3. It sounds like you do not need the nl events at all at this point right.
On 09/13/2017 02:27 PM, Nakayama Kenjiro wrote: > Currently there are no way to disable the netlink sync once enabled > it and even no way to check if it is enable/disabled on the host. You > don't think that it is necessary? At least, I would like to confirm I think your mailer is messing things up and you missed where I said I agree :) See: >>> have to restart the host and never run it again". It sounds like a >>> little bit silly note... >> >> Yes. Agree. >> I just want to make sure I understood your use case and what you really need in case there is a better way to do this. -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/13/2017 12:01 AM, Kenjiro Nakayama wrote: > Currently netlink command reply support option > (TCMU_ATTR_SUPP_KERN_CMD_REPLY) can be enabled only on module > scope. Because of that, once an application enables the netlink > command reply support, all applications using target_core_user.ko > would be expected to support the netlink reply. To make matters worse, > users will not be able to add a device via configfs manually. > > To fix these issues, this patch adds an option to make netlink command > reply disabled on each device through configfs. Original > TCMU_ATTR_SUPP_KERN_CMD_REPLY is still enabled on module scope to keep > backward-compatibility and used by default, however once users set > nl_reply_supported=<NAGATIVE_VALUE> via configfs for a particular > device, the device disables the netlink command reply support. > Patch is ok with me. Reviewed-by: Mike Christie <mchristi@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 942d094269fb..709c27ed4206 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -150,6 +150,8 @@ struct tcmu_dev { wait_queue_head_t nl_cmd_wq; char dev_config[TCMU_CONFIG_LEN]; + + int nl_reply_supported; }; #define TCMU_DEV(_se_dev) container_of(_se_dev, struct tcmu_dev, se_dev) @@ -1306,6 +1308,10 @@ static void tcmu_init_genl_cmd_reply(struct tcmu_dev *udev, int cmd) if (!tcmu_kern_cmd_reply_supported) return; + + if (udev->nl_reply_supported <= 0) + return; + relock: spin_lock(&udev->nl_cmd_lock); @@ -1332,6 +1338,9 @@ static int tcmu_wait_genl_cmd_reply(struct tcmu_dev *udev) if (!tcmu_kern_cmd_reply_supported) return 0; + if (udev->nl_reply_supported <= 0) + return 0; + pr_debug("sleeping for nl reply\n"); wait_for_completion(&nl_cmd->complete); @@ -1506,6 +1515,12 @@ static int tcmu_configure_device(struct se_device *dev) dev->dev_attrib.emulate_write_cache = 0; dev->dev_attrib.hw_queue_depth = 128; + /* If user didn't explicitly disable netlink reply support, use + * module scope setting. + */ + if (udev->nl_reply_supported >= 0) + udev->nl_reply_supported = tcmu_kern_cmd_reply_supported; + /* * Get a ref incase userspace does a close on the uio device before * LIO has initiated tcmu_free_device. @@ -1610,7 +1625,7 @@ static void tcmu_destroy_device(struct se_device *dev) enum { Opt_dev_config, Opt_dev_size, Opt_hw_block_size, Opt_hw_max_sectors, - Opt_err, + Opt_nl_reply_supported, Opt_err, }; static match_table_t tokens = { @@ -1618,6 +1633,7 @@ static match_table_t tokens = { {Opt_dev_size, "dev_size=%u"}, {Opt_hw_block_size, "hw_block_size=%u"}, {Opt_hw_max_sectors, "hw_max_sectors=%u"}, + {Opt_nl_reply_supported, "nl_reply_supported=%d"}, {Opt_err, NULL} }; @@ -1692,6 +1708,18 @@ static ssize_t tcmu_set_configfs_dev_params(struct se_device *dev, ret = tcmu_set_dev_attrib(&args[0], &(dev->dev_attrib.hw_max_sectors)); break; + case Opt_nl_reply_supported: + arg_p = match_strdup(&args[0]); + if (!arg_p) { + ret = -ENOMEM; + break; + } + ret = kstrtol(arg_p, 0, + (long int *) &udev->nl_reply_supported); + kfree(arg_p); + if (ret < 0) + pr_err("kstrtoul() failed for nl_reply_supported=\n"); + break; default: break; } @@ -1842,6 +1870,34 @@ static ssize_t tcmu_dev_size_store(struct config_item *item, const char *page, } CONFIGFS_ATTR(tcmu_, dev_size); +static ssize_t tcmu_nl_reply_supported_show(struct config_item *item, + char *page) +{ + struct se_dev_attrib *da = container_of(to_config_group(item), + struct se_dev_attrib, da_group); + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); + + return snprintf(page, PAGE_SIZE, "%d\n", udev->nl_reply_supported); +} + +static ssize_t tcmu_nl_reply_supported_store(struct config_item *item, + const char *page, size_t count) +{ + struct se_dev_attrib *da = container_of(to_config_group(item), + struct se_dev_attrib, da_group); + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); + s8 val; + int ret; + + ret = kstrtos8(page, 0, &val); + if (ret < 0) + return ret; + + udev->nl_reply_supported = val; + return count; +} +CONFIGFS_ATTR(tcmu_, nl_reply_supported); + static ssize_t tcmu_emulate_write_cache_show(struct config_item *item, char *page) { @@ -1884,6 +1940,7 @@ static struct configfs_attribute *tcmu_attrib_attrs[] = { &tcmu_attr_dev_config, &tcmu_attr_dev_size, &tcmu_attr_emulate_write_cache, + &tcmu_attr_nl_reply_supported, NULL, };
Currently netlink command reply support option (TCMU_ATTR_SUPP_KERN_CMD_REPLY) can be enabled only on module scope. Because of that, once an application enables the netlink command reply support, all applications using target_core_user.ko would be expected to support the netlink reply. To make matters worse, users will not be able to add a device via configfs manually. To fix these issues, this patch adds an option to make netlink command reply disabled on each device through configfs. Original TCMU_ATTR_SUPP_KERN_CMD_REPLY is still enabled on module scope to keep backward-compatibility and used by default, however once users set nl_reply_supported=<NAGATIVE_VALUE> via configfs for a particular device, the device disables the netlink command reply support. Signed-off-by: Kenjiro Nakayama <nakayamakenjiro@gmail.com> --- drivers/target/target_core_user.c | 59 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)