Page MenuHomeFreeBSD
Differential D31809 Diff 101651 tests/sys/ses/destructive.c

Changeset View

Standalone View

View Options

tests/sys/ses/destructive.c

  • This file was added.
/*-
* Copyright (C) 2021 Axcient, Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*
* $FreeBSD$
*/
/* Tests that alter an enclosure's state */
#include <sys/types.h>
#include <sys/ioctl.h>
#include <atf-c.h>
#include <fcntl.h>
#include <glob.h>
#include <regex.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <cam/scsi/scsi_enc.h>
#include "common.h"
// Run a test function on just one ses device
static void
for_one_ses_dev(ses_cb cb)
{
glob_t g;
int fd, r;
g.gl_pathc = 0;
g.gl_pathv = NULL;
g.gl_offs = 0;
r = glob("/dev/ses*", GLOB_NOSORT, NULL, &g);
ATF_REQUIRE_EQ(r, 0);
if (g.gl_pathc == 0)
atf_tc_skip("No ses devices found");
fd = open(g.gl_pathv[0], O_RDWR);
ATF_REQUIRE(fd >= 0);
cb(g.gl_pathv[0], fd);
close(fd);
globfree(&g);
}
static bool do_setelmstat(const char *devname __unused, int fd) {
encioc_element_t *map;
mavUnsubmitted
Not Done
Inline Actions

I am not a "style nazi", but why all one line? Some ATF style?

mav: I am not a "style nazi", but why all one line? Some ATF style?
asomersAuthorUnsubmitted
Done
Inline Actions

I'll change it.

asomers: I'll change it.
unsigned elm_idx;
unsigned nobj;
int r;
elm_type_t last_elm_type = -1;
r = ioctl(fd, ENCIOC_GETNELM, (caddr_t) &nobj);
ATF_REQUIRE_EQ(r, 0);
map = calloc(nobj, sizeof(encioc_element_t));
ATF_REQUIRE(map != NULL);
r = ioctl(fd, ENCIOC_GETELMMAP, (caddr_t) map);
/* Set the IDENT bit for every disk slot */
for (elm_idx = 0; elm_idx < nobj; elm_idx++) {
encioc_elm_status_t elmstat;
struct ses_ctrl_dev_slot *cslot;
struct ses_status_dev_slot *sslot;
if (last_elm_type != map[elm_idx].elm_type) {
/* skip overall elements */
last_elm_type = map[elm_idx].elm_type;
continue;
}
elmstat.elm_idx = elm_idx;
if (map[elm_idx].elm_type == ELMTYP_DEVICE ||
map[elm_idx].elm_type == ELMTYP_ARRAY_DEV)
{
r = ioctl(fd, ENCIOC_GETELMSTAT, (caddr_t)&elmstat);
ATF_REQUIRE_EQ(r, 0);
cslot = (struct ses_ctrl_dev_slot*)&elmstat.cstat[0];
sslot = (struct ses_status_dev_slot*)&elmstat.cstat[0];
ses_ctrl_common_set_select(&cslot->common, 1);
ses_ctrl_dev_slot_set_rqst_ident(cslot, 1);
r = ioctl(fd, ENCIOC_SETELMSTAT, (caddr_t)&elmstat);
ATF_REQUIRE_EQ(r, 0);
mavUnsubmitted
Not Done
Inline Actions

See my recent commit 2e19fae49fd. SES is weird. You can't write status into control on arbitrary hardware.

mav: See my recent commit 2e19fae49fd. SES is weird. You can't write status into control on…
asomersAuthorUnsubmitted
Done
Inline Actions

Ahh, that change crossed this one in review. So I should be good if I run cstat[0] through ses_status_to_ctrl?

asomers: Ahh, that change crossed this one in review. So I should be good if I run `cstat[0]` through…
mavUnsubmitted
Not Done
Inline Actions

As good as possible. I've found there few bits that just can't be read, but has to be set.

mav: As good as possible. I've found there few bits that just can't be read, but has to be set.
}
}
/* Check the IDENT bit for every disk slot */
last_elm_type = -1;
for (elm_idx = 0; elm_idx < nobj; elm_idx++) {
encioc_elm_status_t elmstat;
struct ses_status_dev_slot *sslot;
if (last_elm_type != map[elm_idx].elm_type) {
/* skip overall elements */
last_elm_type = map[elm_idx].elm_type;
continue;
}
elmstat.elm_idx = elm_idx;
if (map[elm_idx].elm_type == ELMTYP_DEVICE ||
map[elm_idx].elm_type == ELMTYP_ARRAY_DEV)
{
int i;
r = ioctl(fd, ENCIOC_GETELMSTAT, (caddr_t)&elmstat);
ATF_REQUIRE_EQ(r, 0);
sslot = (struct ses_status_dev_slot*)&elmstat.cstat[0];
for (i = 0; i < 10; i++) {
r = ioctl(fd, ENCIOC_GETELMSTAT,
(caddr_t)&elmstat);
ATF_REQUIRE_EQ(r, 0);
if (0 == ses_status_dev_slot_get_ident(sslot)) {
/* Needs more time to take effect */
usleep(100000);
}
}
ATF_CHECK(ses_status_dev_slot_get_ident(sslot) != 0);
mavUnsubmitted
Not Done
Inline Actions

This loop is weird. You are reading element status twice before checking its status first time.

Also why do you need the delay? Are there enclosures where ident is not getting back immediately? If so, then read-modify-write attempts are even more hopeless than coming from SES specification.

If the ident got set, does it make sense to read it more? Is it intentional?

mav: This loop is weird. You are reading element status twice before checking its status first time.
asomersAuthorUnsubmitted
Done
Inline Actions

Yeah, I'll remove the first ioctl. As for the loop, I don't recall if I found a SES device that responds slowly, it's been too long since I wrote this test. But there's another reason: I have a WIP change, not yet committed, that makes ENCIOC_SETELMSTAT asynchronous. There are two reasons for that:

  • Using ENCIOC_SETELMSTAT for each element in a large array is very slow, because it can result in a separate SCSI SEND DIAGNOSTIC command for each element. Making the ioctl asynchronous allows the driver to batch them together, sending far fewer SCSI commands.
  • There is a race condition easily triggered by calling ENCIOC_SETELMSTAT back to back on large enclosures, that results in a deadlock. Asynchronous ENCIOC_SETELMSTAT is part of the solution for that race.
asomers: Yeah, I'll remove the first ioctl. As for the loop, I don't recall if I found a SES device…
mavUnsubmitted
Not Done
Inline Actions

I was never happy with present asynchronous reads. Old data they returned were confusing few times. At very least I think it should be redone in some way, may be limiting number of concurrent requests or maximum request rate, but it should not have periodic polling every X seconds.

Asynchronous writes are even more confusing. At very least all asynchronous writes should be allowed to complete before any new reads are allowed, otherwise we'll get response that may not match real hardware one. Present code always forces read-bad after every write for that purpose.

Also some control bits have command semantics, like "do action X", not "set bit to X". Such bits are already weird across read-modify-write, but may become even more weird in case of delayed writes.

mav: I was never happy with present asynchronous reads. Old data they returned were confusing few…
}
}
free(map);
return (true);
}
/*
* sg_ses doesn't provide "dump and restore" functionality. The closest is to
* dump status page 2, then manually edit the file to set every individual
* element select bit, then load the entire file. But that is much too hard.
* Instead, we'll just clear every ident bit.
*/
mavUnsubmitted
Not Done
Inline Actions

It is practically impossible to restore state of some bits, that is probably there is no "dump and restore". See my recent commit 2e19fae49fd. SES is weird.

mav: It is practically impossible to restore state of some bits, that is probably there is no "dump…
static bool
do_setelmstat_cleanup(const char *devname __unused, int fd __unused) {
encioc_element_t *map;
unsigned elm_idx;
unsigned nobj;
int r;
elm_type_t last_elm_type = -1;
r = ioctl(fd, ENCIOC_GETNELM, (caddr_t) &nobj);
ATF_REQUIRE_EQ(r, 0);
map = calloc(nobj, sizeof(encioc_element_t));
ATF_REQUIRE(map != NULL);
r = ioctl(fd, ENCIOC_GETELMMAP, (caddr_t) map);
mavUnsubmitted
Not Done
Inline Actions

Why neither error handling nor assertion? And neither on line 190.

mav: Why neither error handling nor assertion? And neither on line 190.
asomersAuthorUnsubmitted
Done
Inline Actions

I'll add it.

asomers: I'll add it.
/* Clear the IDENT bit for every disk slot */
for (elm_idx = 0; elm_idx < nobj; elm_idx++) {
encioc_elm_status_t elmstat;
struct ses_ctrl_dev_slot *cslot;
if (last_elm_type != map[elm_idx].elm_type) {
/* skip overall elements */
last_elm_type = map[elm_idx].elm_type;
continue;
}
elmstat.elm_idx = elm_idx;
if (map[elm_idx].elm_type == ELMTYP_DEVICE ||
map[elm_idx].elm_type == ELMTYP_ARRAY_DEV)
{
cslot = (struct ses_ctrl_dev_slot*)&elmstat.cstat[0];
ses_ctrl_common_set_select(&cslot->common, 1);
ses_ctrl_dev_slot_set_rqst_ident(cslot, 0);
r = ioctl(fd, ENCIOC_SETELMSTAT, (caddr_t)&elmstat);
}
}
return(true);
}
ATF_TC_WITH_CLEANUP(setelmstat);
ATF_TC_HEAD(setelmstat, tc)
{
atf_tc_set_md_var(tc, "descr", "Exercise ENCIOC_SETELMSTAT");
atf_tc_set_md_var(tc, "require.user", "root");
}
ATF_TC_BODY(setelmstat, tc)
{
for_one_ses_dev(do_setelmstat);
}
ATF_TC_CLEANUP(setelmstat, tc)
{
for_one_ses_dev(do_setelmstat_cleanup);
}
static bool do_setencstat(const char *devname __unused, int fd) {
unsigned char encstat;
int r, i;
bool worked = false;
/*
* SES provides no way to read the current setting of the enclosure
* control page common status bits. So we'll blindly set CRIT.
*/
encstat = 1 << SES_CTRL_PAGE_CRIT_SHIFT;
r = ioctl(fd, ENCIOC_SETENCSTAT, (caddr_t) &encstat);
ATF_REQUIRE_EQ(r, 0);
/* Check that the status has changed */
for (i = 0; i < 10; i++) {
r = ioctl(fd, ENCIOC_GETENCSTAT, (caddr_t) &encstat);
ATF_REQUIRE_EQ(r, 0);
if (encstat & SES_CTRL_PAGE_CRIT_MASK) {
worked = true;
break;
}
usleep(100000);
}
if (!worked) {
/* Some enclosures don't support setting the enclosure status */
return (false);
mavUnsubmitted
Not Done
Inline Actions

Should I implement it for AHCI virtual SES, or what SES hardware this will be run on more often?

mav: Should I implement it for AHCI virtual SES, or what SES hardware this will be run on more often?
asomersAuthorUnsubmitted
Done
Inline Actions

What is AHCI virtual SES? Do you mean SGPIO? I don't think SGPIO is good enough to be of any use for almost anybody. It was my expectation that these tests, when they run at all, would run with regular SAS SES controllers.

asomers: What is AHCI virtual SES? Do you mean SGPIO? I don't think SGPIO is good enough to be of any…
mavUnsubmitted
Not Done
Inline Actions

Yes. While SGPIO is indeed very simplistic, it allows to control few LEDs per drive and present in almost Intel chipset.

mav: Yes. While SGPIO is indeed very simplistic, it allows to control few LEDs per drive and…
} else
return (true);
}
static bool do_setencstat_cleanup(const char *devname __unused, int fd) {
unsigned char encstat;
/*
* SES provides no way to read the current setting of the enclosure
* control page common status bits. So we don't know what they were
* set to before the test. We'll blindly clear all bits.
*/
encstat = 0;
ioctl(fd, ENCIOC_SETENCSTAT, (caddr_t) &encstat);
return (true);
}
ATF_TC_WITH_CLEANUP(setencstat);
ATF_TC_HEAD(setencstat, tc)
{
atf_tc_set_md_var(tc, "descr", "Exercise ENCIOC_SETENCSTAT");
atf_tc_set_md_var(tc, "require.user", "root");
}
ATF_TC_BODY(setencstat, tc)
{
for_each_ses_dev(do_setencstat, O_RDWR);
}
ATF_TC_CLEANUP(setencstat, tc)
{
for_each_ses_dev(do_setencstat_cleanup, O_RDWR);
}
ATF_TP_ADD_TCS(tp)
{
/*
* Untested ioctls:
*
* * ENCIOC_INIT because SES doesn't need it and I don't have any
* SAF-TE devices.
*
* * ENCIOC_SETSTRING because it's seriously unsafe! It's normally
* used for stuff like firmware updates
*/
ATF_TP_ADD_TC(tp, setelmstat);
ATF_TP_ADD_TC(tp, setencstat);
// TODO ENCIOC_SETELMSTAT
mavUnsubmitted
Not Done
Inline Actions

Why TODO, considering setelmstat?

mav: Why TODO, considering setelmstat?
asomersAuthorUnsubmitted
Done
Inline Actions

Why TODO, considering setelmstat?

Oops, I'll remove it.

asomers: > Why TODO, considering setelmstat? Oops, I'll remove it.
return (atf_no_error());
}