thebestvibedev/tromcho.net
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Workaround for Linux bug: return post_op_attr for NFS-RDMA READ3

Browse Source 
Linux NFS RDMA transport has a stupid bug - when the reply doesn't contain
post_op_attr, the data gets offsetted by 84 bytes (size of attributes) and
first 84 bytes are filled with probably random data.
This commit is contained in:
Vitaliy Filippov
2024-12-11 21:09:36 +03:00
parent ae3ca7451f
commit b856524e0c
13 changed files with 112 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/nfs/nfs_block.cpp
+12
View File
@@ -334,10 +334,22 @@ static int block_nfs3_read_proc(void *opaque, rpc_op_t *rop)
}
else
{
nfs_client_t *self = (nfs_client_t*)rop->client;
auto & reply_ok = reply->resok;
// reply_ok.data.data is already set above
reply_ok.count = reply_ok.data.size;
reply_ok.eof = 0;
if (self->rdma_conn)
{
// FIXME Linux NFS RDMA transport has a bug - when the reply
// doesn't contain post_op_attr, the data gets offsetted by
// 84 bytes (size of attributes)...
// So we have to fill it with RDMA. :-(
reply_ok.file_attributes = (post_op_attr){
.attributes_follow = 1,
.attributes = get_file_attributes(self, op->inode),
};
}
}
rpc_queue_reply(rop);
delete op;
src/nfs/nfs_kv.cpp
+29 -3
View File
@@ -65,7 +65,7 @@ int kv_map_type(const std::string & type)
(type == "fifo" ? NF3FIFO : -1)))))));
}
fattr3 get_kv_attributes(nfs_client_t *self, uint64_t ino, json11::Json attrs)
fattr3 get_kv_attributes(nfs_proxy_t *proxy, uint64_t ino, json11::Json attrs)
{
auto type = kv_map_type(attrs["type"].string_value());
auto mode = attrs["mode"].uint64_value();
@@ -86,7 +86,7 @@ fattr3 get_kv_attributes(nfs_client_t *self, uint64_t ino, json11::Json attrs)
.rdev = (type == NF3BLK || type == NF3CHR
? (specdata3){ (uint32_t)attrs["major"].uint64_value(), (uint32_t)attrs["minor"].uint64_value() }
: (specdata3){}),
.fsid = self->parent->fsid,
.fsid = proxy->fsid,
.fileid = ino,
.atime = atime,
.mtime = mtime,
@@ -349,6 +349,27 @@ kv_fs_state_t::~kv_fs_state_t()
}
}
void kv_fs_state_t::write_inode(inode_t ino, json11::Json value, bool hack_cache, std::function<void(int)> cb, std::function<bool(int, const std::string &)> cas_cb)
{
if (!proxy->rdma_context)
{
proxy->db->set(kv_inode_key(ino), value.dump(), cb, cas_cb);
return;
}
// FIXME Linux NFS RDMA transport has a bug - it corrupts the data (by offsetting it 84 bytes)
// when the READ reply doesn't contain post_op_attr. So we have to fill post_op_attr with RDMA. :-(
// So we at least cache it to not repeat K/V requests every read.
read_hack_cache.erase(ino);
proxy->db->set(kv_inode_key(ino), value.dump(), [=](int res)
{
if (hack_cache || res != 0)
read_hack_cache.erase(ino);
else
read_hack_cache[ino] = value;
cb(res);
}, cas_cb);
}
void kv_fs_state_t::update_inode(inode_t ino, bool allow_cache, std::function<void(json11::Json::object &)> change, std::function<void(int)> cb)
{
// FIXME: Use "update" query
@@ -356,12 +377,15 @@ void kv_fs_state_t::update_inode(inode_t ino, bool allow_cache, std::function<vo
{
if (!res)
{
read_hack_cache.erase(ino);
auto ientry = attrs.object_items();
change(ientry);
bool *found = new bool;
*found = true;
proxy->db->set(kv_inode_key(ino), json11::Json(ientry).dump(), [=](int res)
json11::Json ientry_json(ientry);
proxy->db->set(kv_inode_key(ino), ientry_json.dump(), [=](int res)
{
read_hack_cache.erase(ino);
if (!*found)
res = -ENOENT;
delete found;
@@ -384,6 +408,8 @@ void kv_fs_state_t::update_inode(inode_t ino, bool allow_cache, std::function<vo
void kv_fs_state_t::touch_inodes()
{
// Clear RDMA read fattr3 "hack" cache every second
read_hack_cache.clear();
std::set<inode_t> q = std::move(touch_queue);
for (auto ino: q)
{
src/nfs/nfs_kv.h
+3 -1
View File
@@ -75,6 +75,7 @@ struct kv_fs_state_t
std::map<inode_t, kv_inode_extend_t> extends;
std::set<inode_t> touch_queue;
std::map<inode_t, uint64_t> volume_removed;
std::map<inode_t, json11::Json> read_hack_cache;
uint64_t volume_stats_ctr = 0;
uint64_t volume_touch_ctr = 0;
@@ -87,6 +88,7 @@ struct kv_fs_state_t
void upgrade_db(std::function<void(int)> cb);
void defrag_all(json11::Json cfg, std::function<void(int)> cb);
void defrag_volume(inode_t ino, bool no_rm, bool dry_run, std::function<void(int, uint64_t, uint64_t, uint64_t)> cb);
void write_inode(inode_t ino, json11::Json value, bool hack_cache, std::function<void(int)> cb, std::function<bool(int, const std::string &)> cas_cb);
~kv_fs_state_t();
};
@@ -116,7 +118,7 @@ nfstime3 nfstime_from_str(const std::string & s);
std::string nfstime_to_str(nfstime3 t);
std::string nfstime_now_str();
int kv_map_type(const std::string & type);
fattr3 get_kv_attributes(nfs_client_t *self, uint64_t ino, json11::Json attrs);
fattr3 get_kv_attributes(nfs_proxy_t *proxy, uint64_t ino, json11::Json attrs);
std::string kv_direntry_key(uint64_t dir_ino, const std::string & filename);
std::string kv_direntry_filename(const std::string & key);
std::string kv_inode_prefix_key(uint64_t ino, const char *prefix);
src/nfs/nfs_kv_create.cpp
+2 -2
View File
@@ -143,7 +143,7 @@ resume_2:
cb(st->res);
return;
}
st->self->parent->db->set(kv_inode_key(st->new_id), st->attrs.dump().c_str(), [st](int res)
st->self->parent->kvfs->write_inode(st->new_id, st->attrs, false, [st](int res)
{
st->res = res;
kv_continue_create(st, 3);
@@ -267,7 +267,7 @@ template<class T, class Tok> static void kv_create_reply(kv_create_state *st, in
},
.obj_attributes = {
.attributes_follow = 1,
.attributes = get_kv_attributes(st->self, st->new_id, st->attrs),
.attributes = get_kv_attributes(st->self->parent, st->new_id, st->attrs),
},
},
};
src/nfs/nfs_kv_getattr.cpp
+1 -1
View File
@@ -68,7 +68,7 @@ int kv_nfs3_getattr_proc(void *opaque, rpc_op_t *rop)
*reply = (GETATTR3res){
.status = NFS3_OK,
.resok = (GETATTR3resok){
.obj_attributes = get_kv_attributes(self, ino, attrs),
.obj_attributes = get_kv_attributes(self->parent, ino, attrs),
},
};
}
src/nfs/nfs_kv_link.cpp
+2 -2
View File
@@ -102,7 +102,7 @@ resume_2:
new_ientry["ctime"] = nfstime_now_str();
st->ientry = new_ientry;
}
st->self->parent->db->set(kv_inode_key(st->ino), st->ientry.dump(), [st](int res)
st->self->parent->kvfs->write_inode(st->ino, st->ientry, false, [st](int res)
{
st->res = res;
nfs_kv_continue_link(st, 3);
@@ -180,7 +180,7 @@ int kv_nfs3_link_proc(void *opaque, rpc_op_t *rop)
.resok = (LINK3resok){
.file_attributes = (post_op_attr){
.attributes_follow = 1,
.attributes = get_kv_attributes(st->self, st->ino, st->ientry),
.attributes = get_kv_attributes(st->self->parent, st->ino, st->ientry),
},
},
};
src/nfs/nfs_kv_lookup.cpp
+1 -1
View File
@@ -55,7 +55,7 @@ int kv_nfs3_lookup_proc(void *opaque, rpc_op_t *rop)
.object = xdr_copy_string(rop->xdrs, kv_fh(ino)),
.obj_attributes = {
.attributes_follow = 1,
.attributes = get_kv_attributes(self, ino, ientry),
.attributes = get_kv_attributes(self->parent, ino, ientry),
},
},
};
src/nfs/nfs_kv_read.cpp
+41
View File
@@ -37,6 +37,7 @@ static void nfs_kv_continue_read(nfs_kv_read_state *st, int state)
else if (state == 1) goto resume_1;
else if (state == 2) goto resume_2;
else if (state == 3) goto resume_3;
else if (state == 4) goto resume_4;
else
{
fprintf(stderr, "BUG: invalid state in nfs_kv_continue_read()");
@@ -141,6 +142,35 @@ resume_2:
return;
}
}
else if (st->self->rdma_conn)
{
// Take ientry from read_hack_cache for RDMA connections
{
auto rh_it = st->self->parent->kvfs->read_hack_cache.find(st->ino);
if (rh_it != st->self->parent->kvfs->read_hack_cache.end())
{
st->ientry = rh_it->second;
}
}
if (st->ientry.is_null())
{
kv_read_inode(st->self->parent, st->ino, [st](int res, const std::string & value, json11::Json attrs)
{
st->res = res;
st->ientry = attrs;
nfs_kv_continue_read(st, 4);
}, st->allow_cache);
return;
resume_4:
if (st->res < 0 || kv_map_type(st->ientry["type"].string_value()) != NF3REG)
{
auto cb = std::move(st->cb);
cb(st->res < 0 ? st->res : -EINVAL);
return;
}
st->self->parent->kvfs->read_hack_cache[st->ino] = st->ientry;
}
}
st->aligned_offset = align_down(st->offset);
st->aligned_size = align_up(st->offset+st->size) - st->aligned_offset;
assert(!st->aligned_buf);
@@ -198,6 +228,17 @@ int kv_nfs3_read_proc(void *opaque, rpc_op_t *rop)
reply->resok.data.size = st->size;
reply->resok.count = st->size;
reply->resok.eof = st->eof;
if (st->self->rdma_conn)
{
// FIXME Linux NFS RDMA transport has a bug - when the reply
// doesn't contain post_op_attr, the data gets offsetted by
// 84 bytes (size of attributes)...
// So we have to fill it with RDMA. :-(
reply->resok.file_attributes = (post_op_attr){
.attributes_follow = 1,
.attributes = get_kv_attributes(st->self->parent, st->ino, st->ientry),
};
}
}
rpc_queue_reply(st->rop);
delete st;
src/nfs/nfs_kv_readdir.cpp
+3 -3
View File
@@ -57,7 +57,7 @@ static void kv_getattr_next(nfs_kv_readdir_state *st)
st->entries[idx].name_attributes = (post_op_attr){
// FIXME: maybe do not read parent attributes and leave them to a GETATTR?
.attributes_follow = 1,
.attributes = get_kv_attributes(st->self, st->entries[idx].fileid, ientry),
.attributes = get_kv_attributes(st->self->parent, st->entries[idx].fileid, ientry),
};
}
st->getattr_running--;
@@ -126,7 +126,7 @@ resume_1:
dot.fileid = st->dir_ino;
dot.name_attributes = (post_op_attr){
.attributes_follow = 1,
.attributes = get_kv_attributes(st->self, st->dir_ino, st->ientry),
.attributes = get_kv_attributes(st->self->parent, st->dir_ino, st->ientry),
};
dot.name_handle = (post_op_fh3){
.handle_follows = 1,
@@ -169,7 +169,7 @@ resume_2:
dotdot.name_attributes = (post_op_attr){
// FIXME: maybe do not read parent attributes and leave them to a GETATTR?
.attributes_follow = 1,
.attributes = get_kv_attributes(st->self,
.attributes = get_kv_attributes(st->self->parent,
st->parent_ino ? st->parent_ino : st->dir_ino,
st->parent_ino ? st->parent_ientry : st->ientry),
};
src/nfs/nfs_kv_remove.cpp
+1 -1
View File
@@ -197,7 +197,7 @@ resume_5:
auto copy = st->ientry.object_items();
copy["nlink"] = st->ientry["nlink"].uint64_value()-1;
copy["ctime"] = nfstime_now_str();
st->self->parent->db->set(kv_inode_key(st->ino), json11::Json(copy).dump(), [st](int res)
st->self->parent->kvfs->write_inode(st->ino, copy, false, [st](int res)
{
st->res = res;
nfs_kv_continue_delete(st, 6);
src/nfs/nfs_kv_rename.cpp
+2 -2
View File
@@ -240,7 +240,7 @@ resume_7:
copy["nlink"] = st->new_ientry["nlink"].uint64_value()-1;
copy["ctime"] = nfstime_now_str();
copy.erase("verf");
st->self->parent->db->set(kv_inode_key(st->new_direntry["ino"].uint64_value()), json11::Json(copy).dump(), [st](int res)
st->self->parent->kvfs->write_inode(st->new_direntry["ino"].uint64_value(), copy, false, [st](int res)
{
st->res = res;
nfs_kv_continue_rename(st, 8);
@@ -328,7 +328,7 @@ resume_11:
ientry_new["parent_ino"] = st->new_dir_ino;
ientry_new["ctime"] = nfstime_now_str();
ientry_new.erase("verf");
st->self->parent->db->set(kv_inode_key(st->old_direntry["ino"].uint64_value()), json11::Json(ientry_new).dump(), [st](int res)
st->self->parent->kvfs->write_inode(st->old_direntry["ino"].uint64_value(), ientry_new, false, [st](int res)
{
st->res = res;
nfs_kv_continue_rename(st, 12);
src/nfs/nfs_kv_setattr.cpp
+2 -2
View File
@@ -84,7 +84,7 @@ resume_1:
}
st->new_attrs.erase("verf");
st->new_attrs["ctime"] = nfstime_now_str();
st->self->parent->db->set(kv_inode_key(st->ino), json11::Json(st->new_attrs).dump(), [st](int res)
st->self->parent->kvfs->write_inode(st->ino, st->new_attrs, false, [st](int res)
{
st->res = res;
nfs_kv_continue_setattr(st, 2);
@@ -190,7 +190,7 @@ int kv_nfs3_setattr_proc(void *opaque, rpc_op_t *rop)
.obj_wcc = (wcc_data){
.after = (post_op_attr){
.attributes_follow = 1,
.attributes = get_kv_attributes(st->self, st->ino, st->new_attrs),
.attributes = get_kv_attributes(st->self->parent, st->ino, st->new_attrs),
},
},
},
src/nfs/nfs_kv_write.cpp
+13 -13
View File
@@ -553,7 +553,7 @@ static void nfs_do_align_write(nfs_kv_write_state *st, uint64_t ino, uint64_t of
}
}
static std::string new_normal_ientry(nfs_kv_write_state *st)
static json11::Json new_normal_ientry(nfs_kv_write_state *st)
{
auto ni = st->ientry.object_items();
ni.erase("empty");
@@ -564,10 +564,10 @@ static std::string new_normal_ientry(nfs_kv_write_state *st)
ni["size"] = st->ext->cur_extend;
ni["ctime"] = ni["mtime"] = nfstime_now_str();
ni.erase("verf");
return json11::Json(ni).dump();
return ni;
}
static std::string new_moved_ientry(nfs_kv_write_state *st)
static json11::Json new_moved_ientry(nfs_kv_write_state *st)
{
auto ni = st->ientry.object_items();
ni.erase("empty");
@@ -578,10 +578,10 @@ static std::string new_moved_ientry(nfs_kv_write_state *st)
ni["size"] = st->new_size;
ni["ctime"] = ni["mtime"] = nfstime_now_str();
ni.erase("verf");
return json11::Json(ni).dump();
return ni;
}
static std::string new_shared_ientry(nfs_kv_write_state *st)
static json11::Json new_shared_ientry(nfs_kv_write_state *st)
{
auto ni = st->ientry.object_items();
ni.erase("empty");
@@ -589,10 +589,10 @@ static std::string new_shared_ientry(nfs_kv_write_state *st)
ni["ctime"] = ni["mtime"] = nfstime_now_str();
ni["shared_ver"] = ni["shared_ver"].uint64_value()+1;
ni.erase("verf");
return json11::Json(ni).dump();
return ni;
}
static std::string new_unshared_ientry(nfs_kv_write_state *st)
static json11::Json new_unshared_ientry(nfs_kv_write_state *st)
{
auto ni = st->ientry.object_items();
ni.erase("empty");
@@ -602,7 +602,7 @@ static std::string new_unshared_ientry(nfs_kv_write_state *st)
ni.erase("shared_ver");
ni["ctime"] = ni["mtime"] = nfstime_now_str();
ni.erase("verf");
return json11::Json(ni).dump();
return ni;
}
static void nfs_kv_extend_inode(nfs_kv_write_state *st, int state, int base_state)
@@ -612,7 +612,7 @@ static void nfs_kv_extend_inode(nfs_kv_write_state *st, int state, int base_stat
st->ext->cur_extend = st->ext->next_extend;
st->ext->next_extend = 0;
st->res2 = -EAGAIN;
st->proxy->db->set(kv_inode_key(st->ino), new_normal_ientry(st), [st, base_state](int res)
st->proxy->kvfs->write_inode(st->ino, new_normal_ientry(st), true, [st, base_state](int res)
{
st->res = res;
nfs_kv_continue_write(st, base_state+1);
@@ -838,7 +838,7 @@ resume_4:
cb(st->res);
return;
}
st->proxy->db->set(kv_inode_key(st->ino), new_moved_ientry(st), [st](int res)
st->proxy->kvfs->write_inode(st->ino, new_moved_ientry(st), true, [st](int res)
{
st->res = res;
nfs_kv_continue_write(st, 5);
@@ -881,7 +881,7 @@ resume_7:
}
resume_8:
// We always have to change inode entry on shared writes
st->proxy->db->set(kv_inode_key(st->ino), new_shared_ientry(st), [st](int res)
st->proxy->kvfs->write_inode(st->ino, new_shared_ientry(st), true, [st](int res)
{
st->res = res;
nfs_kv_continue_write(st, 9);
@@ -930,7 +930,7 @@ resume_11:
return;
}
}
st->proxy->db->set(kv_inode_key(st->ino), new_unshared_ientry(st), [st](int res)
st->proxy->kvfs->write_inode(st->ino, new_unshared_ientry(st), true, [st](int res)
{
st->res = res;
nfs_kv_continue_write(st, 12);
@@ -953,7 +953,7 @@ resume_12:
}
// Record removed part of the shared inode as obsolete in statistics
st->proxy->kvfs->volume_removed[st->ientry["shared_ino"].uint64_value()] += st->ientry["shared_alloc"].uint64_value();
st->ientry_text = new_unshared_ientry(st);
st->ientry_text = new_unshared_ientry(st).dump();
}
// Non-shared write
nfs_do_align_write(st, st->ino, st->offset, 0, 13);