Page MenuHomeFreeBSD
Differential D29075

Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver
ClosedPublic
Actions

Authored by whu on Mar 5 2021, 9:55 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 6, 6:38 PM
Unknown Object (File)
Fri, Mar 28, 9:08 PM
Unknown Object (File)
Thu, Mar 27, 9:17 AM
Unknown Object (File)
Wed, Mar 26, 12:25 PM
Unknown Object (File)
Sat, Mar 15, 1:16 AM
Unknown Object (File)
Feb 21 2025, 6:11 PM
Unknown Object (File)
Feb 17 2025, 8:34 AM
Unknown Object (File)
Jan 31 2025, 8:53 AM
View All Files
Subscribers
ae
franco_opnsense.org
glebius
imp
lwhsu
melifaro

Details

Reviewers
decui_microsoft.com
Commits
rGcdc59163ff8e: Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver
rG4150446f6467: Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver
rGa491581f3f8d: Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver
Summary

Receive Segment Coalescing (RSC) in the vSwitch is a feature available in
Windows Server 2019 hosts and later. It reduces the per packet processing
overhead by coalescing multiple TCP segments when possible. This happens
mostly when TCP traffics are among different guests on same host.
This patch adds netvsc driver support for this feature.

The patch also updates NVS version to 6.1 as needed for RSC
enablement.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

whu requested review of this revision.Mar 5 2021, 9:55 AM
whu created this revision.
lwhsu added a subscriber: lwhsu.Mar 6 2021, 2:50 AM
whu updated this revision to Diff 85563.Mar 11 2021, 10:03 AM
whu edited the summary of this revision. (Show Details)
whu added a reviewer: decui_microsoft.com.
Herald added subscribers: melifaro, ae. · View Herald TranscriptMar 11 2021, 10:03 AM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 12 2021, 4:44 AM
Closed by commit rGa491581f3f8d: Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver (authored by whu). · Explain Why
This revision was automatically updated to reflect the committed changes.
whu added a commit: rGa491581f3f8d: Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver.
whu added a commit: rG4150446f6467: Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver.Mar 29 2021, 9:13 AM
whu added a commit: rGcdc59163ff8e: Hyper-V: hn: Enable vSwitch RSC support in hn netvsc driver.Mar 29 2021, 10:21 AM
franco_opnsense.org added a subscriber: franco_opnsense.org.Jan 31 2022, 6:44 AM

Hello @whu @lwhsu @decui_microsoft.com,

We have multiple reports that this causes throughput regressions when in use on 13-STABLE as opposed to 13.0-RELEASE where it is not present. We have had this commit reverted and speeds are back to normal for our OPNsense users. For more info see https://forum.opnsense.org/index.php?topic=26364.0

Cheers,
Franco

Herald added subscribers: glebius, imp. · View Herald TranscriptJan 31 2022, 6:44 AM
whu added a comment.Feb 1 2022, 8:01 AM
In D29075#771330, @franco_opnsense.org wrote:

Hello @whu @lwhsu @decui_microsoft.com,

We have multiple reports that this causes throughput regressions when in use on 13-STABLE as opposed to 13.0-RELEASE where it is not present. We have had this commit reverted and speeds are back to normal for our OPNsense users. For more info see https://forum.opnsense.org/index.php?topic=26364.0

Cheers,
Franco

Franco,
Thanks for reporting this. I looked that the thread on the forum. But also please help answering a few questions below for me.

  1. Are you seeing the issue on on-prem Hyper-V or in Azure or both? If on Hyper-V, please provide the Windows version. If in Azure, please provide the VM size/type information.
  2. What kind of network traffic are you seeing the throughput issue, tcp or udp or both? What are the throughput numbers with and without this issue?
  3. Are the sender and receiver running on the same Hyper-V host? You probably can only tell this if the problem also presents in on-prem environment.
  4. When the problem presents, what are the outputs for "sysctl -a | grep rsc" on the VM? I am interested in the outputs of rsc_drop, rsc_pkts on each channel to see if there are anything wrong.
  5. It seems you are seeing it on OPNsense. Do you have a reproducible case on FreeBSD 13-stable? It will help me to further troubleshoot.

Please cc to weh@microsoft.com or whu@freebsd.org for further discussion on this case. This review board is not closely monitored.

Thanks,
Wei

Revision Contents
Changeset List

PathSize
sys/
dev/
hyperv/
netvsc/
5 lines
25 lines
258 lines
14 lines
19 lines
31 lines
net/
7 lines

Diff 85563

View Options

sys/dev/hyperv/netvsc/hn_nvs.c

Show First 20 Lines • Show All 182 Lines • ▼ Show 20 Lines union {
} vop_args; } vop_args;
} args; } args;
}; };
static int vn_io_fault1(struct vnode *vp, struct uio *uio, static int vn_io_fault1(struct vnode *vp, struct uio *uio,
struct vn_io_fault_args *args, struct thread *td); struct vn_io_fault_args *args, struct thread *td);
int int
vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp) vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp)
kibUnsubmitted
Not Done
Inline Actions

Why do you need this?

For rw it was needed due to the vn_io_fault() structure, for vn_deallocate() it is pointless complication.

kib: Why do you need this? For rw it was needed due to the vn_io_fault() structure, for…
{ {
struct thread *td = ndp->ni_cnd.cn_thread; struct thread *td = ndp->ni_cnd.cn_thread;
return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp)); return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp));
} }
static uint64_t static uint64_t
open2nameif(int fmode, u_int vn_open_flags) open2nameif(int fmode, u_int vn_open_flags)
▲ Show 20 Lines • Show All 3,260 Lines • ▼ Show 20 Lines#endif
return (error); return (error);
} }
static int static int
vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags,
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
struct ucred *cred;
struct mount *mp; struct mount *mp;
void *rl_cookie; void *rl_cookie;
off_t off, len; off_t off, len;
int error; int error;
#ifdef AUDIT #ifdef AUDIT
bool audited_vnode1 = false; bool audited_vnode1 = false;
#endif #endif
Show All 27 Lines#ifdef AUDIT
} }
#endif #endif
#ifdef MAC #ifdef MAC
if ((ioflag & IO_NOMACCHECK) == 0) if ((ioflag & IO_NOMACCHECK) == 0)
error = mac_vnode_check_write(active_cred, file_cred, error = mac_vnode_check_write(active_cred, file_cred,
vp); vp);
#endif #endif
if (error == 0) if (error == 0) {
if (file_cred != NULL)
cred = file_cred;
else
cred = active_cred;
error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag, error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag,
active_cred); cred);
}
if ((ioflag & IO_NODELOCKED) == 0) { if ((ioflag & IO_NODELOCKED) == 0) {
VOP_UNLOCK(vp); VOP_UNLOCK(vp);
if (mp != NULL) { if (mp != NULL) {
vn_finished_write(mp); vn_finished_write(mp);
mp = NULL; mp = NULL;
} }
} }
if (error == 0 && len != 0) if (error == 0 && len != 0)
maybe_yield(); maybe_yield();
} }
out: out:
if (rl_cookie != NULL) if (rl_cookie != NULL)
vn_rangelock_unlock(vp, rl_cookie); vn_rangelock_unlock(vp, rl_cookie);
*offset = off; *offset = off;
*length = len; *length = len;
return (error); return (error);
} }
int int
vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags,
kibUnsubmitted
Not Done
Inline Actions

When I talked about commenting the use, I mean that it should explain that function is supposed to be used in the situations where the op is not triggered by a user request. In other words, currthread->td_ucred == active_cred is generally not the right credentials to account the operation to.

Your line above just repeats the code.

kib: When I talked about commenting the use, I mean that it should explain that function is supposed…
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset || if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset ||
flags != 0) flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
return (vn_deallocate_impl(vp, offset, length, flags, ioflag, return (vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, file_cred)); active_cred, file_cred));
} }
kibUnsubmitted
Done
Inline Actions

cred = file_cred != NOCRED ? file_cred : active_cred;

kib: `cred = file_cred != NOCRED ? file_cred : active_cred;`
static int static int
vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags, vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags,
struct ucred *active_cred, struct thread *td) struct ucred *active_cred, struct thread *td)
{ {
int error; int error;
struct vnode *vp; struct vnode *vp;
int ioflag; int ioflag;
vp = fp->f_vnode; vp = fp->f_vnode;
if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 || if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 ||
*length > OFF_MAX - *offset || flags != 0) *length > OFF_MAX - *offset || flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
ioflag = get_write_ioflag(fp); ioflag = get_write_ioflag(fp);
switch (cmd) { switch (cmd) {
case SPACECTL_DEALLOC: case SPACECTL_DEALLOC:
error = vn_deallocate_impl(vp, offset, length, flags, ioflag, error = vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, fp->f_cred); active_cred, fp->f_cred);
kibUnsubmitted
Not Done
Inline Actions

why passing fp->f_cred there at all?
from_fops is ugly and arguably not needed.
vn_deallocate() needs explanation about active_cre/file_cred in a comment, nobody would read man page for this (or in both)

kib: why passing fp->f_cred there at all? from_fops is ugly and arguably not needed. vn_deallocate()…
khngAuthorUnsubmitted
Not Done
Inline Actions

The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the policy modules in base so far are not taking f_cred into consideration at all).

khng: The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the…
khngAuthorUnsubmitted
Done
Inline Actions

A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base policy module, it is possibly used by outside world to implement capability semantic.

khng: A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base…
break; break;
default: default:
panic("vn_fspacectl: unknown cmd %d", cmd); panic("vn_fspacectl: unknown cmd %d", cmd);
} }
return (error); return (error);
} }
▲ Show 20 Lines • Show All 101 Lines • Show Last 20 Lines
View Options

sys/dev/hyperv/netvsc/hn_rndis.c

Show First 20 Lines • Show All 182 Lines • ▼ Show 20 Lines union {
} vop_args; } vop_args;
} args; } args;
}; };
static int vn_io_fault1(struct vnode *vp, struct uio *uio, static int vn_io_fault1(struct vnode *vp, struct uio *uio,
struct vn_io_fault_args *args, struct thread *td); struct vn_io_fault_args *args, struct thread *td);
int int
vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp) vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp)
kibUnsubmitted
Not Done
Inline Actions

Why do you need this?

For rw it was needed due to the vn_io_fault() structure, for vn_deallocate() it is pointless complication.

kib: Why do you need this? For rw it was needed due to the vn_io_fault() structure, for…
{ {
struct thread *td = ndp->ni_cnd.cn_thread; struct thread *td = ndp->ni_cnd.cn_thread;
return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp)); return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp));
} }
static uint64_t static uint64_t
open2nameif(int fmode, u_int vn_open_flags) open2nameif(int fmode, u_int vn_open_flags)
▲ Show 20 Lines • Show All 3,260 Lines • ▼ Show 20 Lines#endif
return (error); return (error);
} }
static int static int
vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags,
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
struct ucred *cred;
struct mount *mp; struct mount *mp;
void *rl_cookie; void *rl_cookie;
off_t off, len; off_t off, len;
int error; int error;
#ifdef AUDIT #ifdef AUDIT
bool audited_vnode1 = false; bool audited_vnode1 = false;
#endif #endif
Show All 27 Lines#ifdef AUDIT
} }
#endif #endif
#ifdef MAC #ifdef MAC
if ((ioflag & IO_NOMACCHECK) == 0) if ((ioflag & IO_NOMACCHECK) == 0)
error = mac_vnode_check_write(active_cred, file_cred, error = mac_vnode_check_write(active_cred, file_cred,
vp); vp);
#endif #endif
if (error == 0) if (error == 0) {
if (file_cred != NULL)
cred = file_cred;
else
cred = active_cred;
error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag, error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag,
active_cred); cred);
}
if ((ioflag & IO_NODELOCKED) == 0) { if ((ioflag & IO_NODELOCKED) == 0) {
VOP_UNLOCK(vp); VOP_UNLOCK(vp);
if (mp != NULL) { if (mp != NULL) {
vn_finished_write(mp); vn_finished_write(mp);
mp = NULL; mp = NULL;
} }
} }
if (error == 0 && len != 0) if (error == 0 && len != 0)
maybe_yield(); maybe_yield();
} }
out: out:
if (rl_cookie != NULL) if (rl_cookie != NULL)
vn_rangelock_unlock(vp, rl_cookie); vn_rangelock_unlock(vp, rl_cookie);
*offset = off; *offset = off;
*length = len; *length = len;
return (error); return (error);
} }
int int
vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags,
kibUnsubmitted
Not Done
Inline Actions

When I talked about commenting the use, I mean that it should explain that function is supposed to be used in the situations where the op is not triggered by a user request. In other words, currthread->td_ucred == active_cred is generally not the right credentials to account the operation to.

Your line above just repeats the code.

kib: When I talked about commenting the use, I mean that it should explain that function is supposed…
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset || if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset ||
flags != 0) flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
return (vn_deallocate_impl(vp, offset, length, flags, ioflag, return (vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, file_cred)); active_cred, file_cred));
} }
kibUnsubmitted
Done
Inline Actions

cred = file_cred != NOCRED ? file_cred : active_cred;

kib: `cred = file_cred != NOCRED ? file_cred : active_cred;`
static int static int
vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags, vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags,
struct ucred *active_cred, struct thread *td) struct ucred *active_cred, struct thread *td)
{ {
int error; int error;
struct vnode *vp; struct vnode *vp;
int ioflag; int ioflag;
vp = fp->f_vnode; vp = fp->f_vnode;
if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 || if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 ||
*length > OFF_MAX - *offset || flags != 0) *length > OFF_MAX - *offset || flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
ioflag = get_write_ioflag(fp); ioflag = get_write_ioflag(fp);
switch (cmd) { switch (cmd) {
case SPACECTL_DEALLOC: case SPACECTL_DEALLOC:
error = vn_deallocate_impl(vp, offset, length, flags, ioflag, error = vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, fp->f_cred); active_cred, fp->f_cred);
kibUnsubmitted
Not Done
Inline Actions

why passing fp->f_cred there at all?
from_fops is ugly and arguably not needed.
vn_deallocate() needs explanation about active_cre/file_cred in a comment, nobody would read man page for this (or in both)

kib: why passing fp->f_cred there at all? from_fops is ugly and arguably not needed. vn_deallocate()…
khngAuthorUnsubmitted
Not Done
Inline Actions

The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the policy modules in base so far are not taking f_cred into consideration at all).

khng: The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the…
khngAuthorUnsubmitted
Done
Inline Actions

A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base policy module, it is possibly used by outside world to implement capability semantic.

khng: A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base…
break; break;
default: default:
panic("vn_fspacectl: unknown cmd %d", cmd); panic("vn_fspacectl: unknown cmd %d", cmd);
} }
return (error); return (error);
} }
▲ Show 20 Lines • Show All 101 Lines • Show Last 20 Lines
View Options

sys/dev/hyperv/netvsc/if_hn.c

Show First 20 Lines • Show All 182 Lines • ▼ Show 20 Lines union {
} vop_args; } vop_args;
} args; } args;
}; };
static int vn_io_fault1(struct vnode *vp, struct uio *uio, static int vn_io_fault1(struct vnode *vp, struct uio *uio,
struct vn_io_fault_args *args, struct thread *td); struct vn_io_fault_args *args, struct thread *td);
int int
vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp) vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp)
kibUnsubmitted
Not Done
Inline Actions

Why do you need this?

For rw it was needed due to the vn_io_fault() structure, for vn_deallocate() it is pointless complication.

kib: Why do you need this? For rw it was needed due to the vn_io_fault() structure, for…
{ {
struct thread *td = ndp->ni_cnd.cn_thread; struct thread *td = ndp->ni_cnd.cn_thread;
return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp)); return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp));
} }
static uint64_t static uint64_t
open2nameif(int fmode, u_int vn_open_flags) open2nameif(int fmode, u_int vn_open_flags)
▲ Show 20 Lines • Show All 3,260 Lines • ▼ Show 20 Lines#endif
return (error); return (error);
} }
static int static int
vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags,
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
struct ucred *cred;
struct mount *mp; struct mount *mp;
void *rl_cookie; void *rl_cookie;
off_t off, len; off_t off, len;
int error; int error;
#ifdef AUDIT #ifdef AUDIT
bool audited_vnode1 = false; bool audited_vnode1 = false;
#endif #endif
Show All 27 Lines#ifdef AUDIT
} }
#endif #endif
#ifdef MAC #ifdef MAC
if ((ioflag & IO_NOMACCHECK) == 0) if ((ioflag & IO_NOMACCHECK) == 0)
error = mac_vnode_check_write(active_cred, file_cred, error = mac_vnode_check_write(active_cred, file_cred,
vp); vp);
#endif #endif
if (error == 0) if (error == 0) {
if (file_cred != NULL)
cred = file_cred;
else
cred = active_cred;
error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag, error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag,
active_cred); cred);
}
if ((ioflag & IO_NODELOCKED) == 0) { if ((ioflag & IO_NODELOCKED) == 0) {
VOP_UNLOCK(vp); VOP_UNLOCK(vp);
if (mp != NULL) { if (mp != NULL) {
vn_finished_write(mp); vn_finished_write(mp);
mp = NULL; mp = NULL;
} }
} }
if (error == 0 && len != 0) if (error == 0 && len != 0)
maybe_yield(); maybe_yield();
} }
out: out:
if (rl_cookie != NULL) if (rl_cookie != NULL)
vn_rangelock_unlock(vp, rl_cookie); vn_rangelock_unlock(vp, rl_cookie);
*offset = off; *offset = off;
*length = len; *length = len;
return (error); return (error);
} }
int int
vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags,
kibUnsubmitted
Not Done
Inline Actions

When I talked about commenting the use, I mean that it should explain that function is supposed to be used in the situations where the op is not triggered by a user request. In other words, currthread->td_ucred == active_cred is generally not the right credentials to account the operation to.

Your line above just repeats the code.

kib: When I talked about commenting the use, I mean that it should explain that function is supposed…
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset || if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset ||
flags != 0) flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
return (vn_deallocate_impl(vp, offset, length, flags, ioflag, return (vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, file_cred)); active_cred, file_cred));
} }
kibUnsubmitted
Done
Inline Actions

cred = file_cred != NOCRED ? file_cred : active_cred;

kib: `cred = file_cred != NOCRED ? file_cred : active_cred;`
static int static int
vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags, vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags,
struct ucred *active_cred, struct thread *td) struct ucred *active_cred, struct thread *td)
{ {
int error; int error;
struct vnode *vp; struct vnode *vp;
int ioflag; int ioflag;
vp = fp->f_vnode; vp = fp->f_vnode;
if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 || if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 ||
*length > OFF_MAX - *offset || flags != 0) *length > OFF_MAX - *offset || flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
ioflag = get_write_ioflag(fp); ioflag = get_write_ioflag(fp);
switch (cmd) { switch (cmd) {
case SPACECTL_DEALLOC: case SPACECTL_DEALLOC:
error = vn_deallocate_impl(vp, offset, length, flags, ioflag, error = vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, fp->f_cred); active_cred, fp->f_cred);
kibUnsubmitted
Not Done
Inline Actions

why passing fp->f_cred there at all?
from_fops is ugly and arguably not needed.
vn_deallocate() needs explanation about active_cre/file_cred in a comment, nobody would read man page for this (or in both)

kib: why passing fp->f_cred there at all? from_fops is ugly and arguably not needed. vn_deallocate()…
khngAuthorUnsubmitted
Not Done
Inline Actions

The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the policy modules in base so far are not taking f_cred into consideration at all).

khng: The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the…
khngAuthorUnsubmitted
Done
Inline Actions

A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base policy module, it is possibly used by outside world to implement capability semantic.

khng: A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base…
break; break;
default: default:
panic("vn_fspacectl: unknown cmd %d", cmd); panic("vn_fspacectl: unknown cmd %d", cmd);
} }
return (error); return (error);
} }
▲ Show 20 Lines • Show All 101 Lines • Show Last 20 Lines
View Options

sys/dev/hyperv/netvsc/if_hnreg.h

Show First 20 Lines • Show All 182 Lines • ▼ Show 20 Lines union {
} vop_args; } vop_args;
} args; } args;
}; };
static int vn_io_fault1(struct vnode *vp, struct uio *uio, static int vn_io_fault1(struct vnode *vp, struct uio *uio,
struct vn_io_fault_args *args, struct thread *td); struct vn_io_fault_args *args, struct thread *td);
int int
vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp) vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp)
kibUnsubmitted
Not Done
Inline Actions

Why do you need this?

For rw it was needed due to the vn_io_fault() structure, for vn_deallocate() it is pointless complication.

kib: Why do you need this? For rw it was needed due to the vn_io_fault() structure, for…
{ {
struct thread *td = ndp->ni_cnd.cn_thread; struct thread *td = ndp->ni_cnd.cn_thread;
return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp)); return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp));
} }
static uint64_t static uint64_t
open2nameif(int fmode, u_int vn_open_flags) open2nameif(int fmode, u_int vn_open_flags)
▲ Show 20 Lines • Show All 3,260 Lines • ▼ Show 20 Lines#endif
return (error); return (error);
} }
static int static int
vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags,
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
struct ucred *cred;
struct mount *mp; struct mount *mp;
void *rl_cookie; void *rl_cookie;
off_t off, len; off_t off, len;
int error; int error;
#ifdef AUDIT #ifdef AUDIT
bool audited_vnode1 = false; bool audited_vnode1 = false;
#endif #endif
Show All 27 Lines#ifdef AUDIT
} }
#endif #endif
#ifdef MAC #ifdef MAC
if ((ioflag & IO_NOMACCHECK) == 0) if ((ioflag & IO_NOMACCHECK) == 0)
error = mac_vnode_check_write(active_cred, file_cred, error = mac_vnode_check_write(active_cred, file_cred,
vp); vp);
#endif #endif
if (error == 0) if (error == 0) {
if (file_cred != NULL)
cred = file_cred;
else
cred = active_cred;
error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag, error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag,
active_cred); cred);
}
if ((ioflag & IO_NODELOCKED) == 0) { if ((ioflag & IO_NODELOCKED) == 0) {
VOP_UNLOCK(vp); VOP_UNLOCK(vp);
if (mp != NULL) { if (mp != NULL) {
vn_finished_write(mp); vn_finished_write(mp);
mp = NULL; mp = NULL;
} }
} }
if (error == 0 && len != 0) if (error == 0 && len != 0)
maybe_yield(); maybe_yield();
} }
out: out:
if (rl_cookie != NULL) if (rl_cookie != NULL)
vn_rangelock_unlock(vp, rl_cookie); vn_rangelock_unlock(vp, rl_cookie);
*offset = off; *offset = off;
*length = len; *length = len;
return (error); return (error);
} }
int int
vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags,
kibUnsubmitted
Not Done
Inline Actions

When I talked about commenting the use, I mean that it should explain that function is supposed to be used in the situations where the op is not triggered by a user request. In other words, currthread->td_ucred == active_cred is generally not the right credentials to account the operation to.

Your line above just repeats the code.

kib: When I talked about commenting the use, I mean that it should explain that function is supposed…
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset || if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset ||
flags != 0) flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
return (vn_deallocate_impl(vp, offset, length, flags, ioflag, return (vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, file_cred)); active_cred, file_cred));
} }
kibUnsubmitted
Done
Inline Actions

cred = file_cred != NOCRED ? file_cred : active_cred;

kib: `cred = file_cred != NOCRED ? file_cred : active_cred;`
static int static int
vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags, vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags,
struct ucred *active_cred, struct thread *td) struct ucred *active_cred, struct thread *td)
{ {
int error; int error;
struct vnode *vp; struct vnode *vp;
int ioflag; int ioflag;
vp = fp->f_vnode; vp = fp->f_vnode;
if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 || if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 ||
*length > OFF_MAX - *offset || flags != 0) *length > OFF_MAX - *offset || flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
ioflag = get_write_ioflag(fp); ioflag = get_write_ioflag(fp);
switch (cmd) { switch (cmd) {
case SPACECTL_DEALLOC: case SPACECTL_DEALLOC:
error = vn_deallocate_impl(vp, offset, length, flags, ioflag, error = vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, fp->f_cred); active_cred, fp->f_cred);
kibUnsubmitted
Not Done
Inline Actions

why passing fp->f_cred there at all?
from_fops is ugly and arguably not needed.
vn_deallocate() needs explanation about active_cre/file_cred in a comment, nobody would read man page for this (or in both)

kib: why passing fp->f_cred there at all? from_fops is ugly and arguably not needed. vn_deallocate()…
khngAuthorUnsubmitted
Not Done
Inline Actions

The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the policy modules in base so far are not taking f_cred into consideration at all).

khng: The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the…
khngAuthorUnsubmitted
Done
Inline Actions

A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base policy module, it is possibly used by outside world to implement capability semantic.

khng: A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base…
break; break;
default: default:
panic("vn_fspacectl: unknown cmd %d", cmd); panic("vn_fspacectl: unknown cmd %d", cmd);
} }
return (error); return (error);
} }
▲ Show 20 Lines • Show All 101 Lines • Show Last 20 Lines
View Options

sys/dev/hyperv/netvsc/if_hnvar.h

Show First 20 Lines • Show All 182 Lines • ▼ Show 20 Lines union {
} vop_args; } vop_args;
} args; } args;
}; };
static int vn_io_fault1(struct vnode *vp, struct uio *uio, static int vn_io_fault1(struct vnode *vp, struct uio *uio,
struct vn_io_fault_args *args, struct thread *td); struct vn_io_fault_args *args, struct thread *td);
int int
vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp) vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp)
kibUnsubmitted
Not Done
Inline Actions

Why do you need this?

For rw it was needed due to the vn_io_fault() structure, for vn_deallocate() it is pointless complication.

kib: Why do you need this? For rw it was needed due to the vn_io_fault() structure, for…
{ {
struct thread *td = ndp->ni_cnd.cn_thread; struct thread *td = ndp->ni_cnd.cn_thread;
return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp)); return (vn_open_cred(ndp, flagp, cmode, 0, td->td_ucred, fp));
} }
static uint64_t static uint64_t
open2nameif(int fmode, u_int vn_open_flags) open2nameif(int fmode, u_int vn_open_flags)
▲ Show 20 Lines • Show All 3,260 Lines • ▼ Show 20 Lines#endif
return (error); return (error);
} }
static int static int
vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate_impl(struct vnode *vp, off_t *offset, off_t *length, int flags,
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
struct ucred *cred;
struct mount *mp; struct mount *mp;
void *rl_cookie; void *rl_cookie;
off_t off, len; off_t off, len;
int error; int error;
#ifdef AUDIT #ifdef AUDIT
bool audited_vnode1 = false; bool audited_vnode1 = false;
#endif #endif
Show All 27 Lines#ifdef AUDIT
} }
#endif #endif
#ifdef MAC #ifdef MAC
if ((ioflag & IO_NOMACCHECK) == 0) if ((ioflag & IO_NOMACCHECK) == 0)
error = mac_vnode_check_write(active_cred, file_cred, error = mac_vnode_check_write(active_cred, file_cred,
vp); vp);
#endif #endif
if (error == 0) if (error == 0) {
if (file_cred != NULL)
cred = file_cred;
else
cred = active_cred;
error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag, error = VOP_DEALLOCATE(vp, &off, &len, flags, ioflag,
active_cred); cred);
}
if ((ioflag & IO_NODELOCKED) == 0) { if ((ioflag & IO_NODELOCKED) == 0) {
VOP_UNLOCK(vp); VOP_UNLOCK(vp);
if (mp != NULL) { if (mp != NULL) {
vn_finished_write(mp); vn_finished_write(mp);
mp = NULL; mp = NULL;
} }
} }
if (error == 0 && len != 0) if (error == 0 && len != 0)
maybe_yield(); maybe_yield();
} }
out: out:
if (rl_cookie != NULL) if (rl_cookie != NULL)
vn_rangelock_unlock(vp, rl_cookie); vn_rangelock_unlock(vp, rl_cookie);
*offset = off; *offset = off;
*length = len; *length = len;
return (error); return (error);
} }
int int
vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags, vn_deallocate(struct vnode *vp, off_t *offset, off_t *length, int flags,
kibUnsubmitted
Not Done
Inline Actions

When I talked about commenting the use, I mean that it should explain that function is supposed to be used in the situations where the op is not triggered by a user request. In other words, currthread->td_ucred == active_cred is generally not the right credentials to account the operation to.

Your line above just repeats the code.

kib: When I talked about commenting the use, I mean that it should explain that function is supposed…
int ioflag, struct ucred *active_cred, struct ucred *file_cred) int ioflag, struct ucred *active_cred, struct ucred *file_cred)
{ {
if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset || if (*offset < 0 || *length <= 0 || *length > OFF_MAX - *offset ||
flags != 0) flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
return (vn_deallocate_impl(vp, offset, length, flags, ioflag, return (vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, file_cred)); active_cred, file_cred));
} }
kibUnsubmitted
Done
Inline Actions

cred = file_cred != NOCRED ? file_cred : active_cred;

kib: `cred = file_cred != NOCRED ? file_cred : active_cred;`
static int static int
vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags, vn_fspacectl(struct file *fp, int cmd, off_t *offset, off_t *length, int flags,
struct ucred *active_cred, struct thread *td) struct ucred *active_cred, struct thread *td)
{ {
int error; int error;
struct vnode *vp; struct vnode *vp;
int ioflag; int ioflag;
vp = fp->f_vnode; vp = fp->f_vnode;
if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 || if (cmd != SPACECTL_DEALLOC || *offset < 0 || *length <= 0 ||
*length > OFF_MAX - *offset || flags != 0) *length > OFF_MAX - *offset || flags != 0)
return (EINVAL); return (EINVAL);
if (vp->v_type != VREG) if (vp->v_type != VREG)
return (ENODEV); return (ENODEV);
ioflag = get_write_ioflag(fp); ioflag = get_write_ioflag(fp);
switch (cmd) { switch (cmd) {
case SPACECTL_DEALLOC: case SPACECTL_DEALLOC:
error = vn_deallocate_impl(vp, offset, length, flags, ioflag, error = vn_deallocate_impl(vp, offset, length, flags, ioflag,
active_cred, fp->f_cred); active_cred, fp->f_cred);
kibUnsubmitted
Not Done
Inline Actions

why passing fp->f_cred there at all?
from_fops is ugly and arguably not needed.
vn_deallocate() needs explanation about active_cre/file_cred in a comment, nobody would read man page for this (or in both)

kib: why passing fp->f_cred there at all? from_fops is ugly and arguably not needed. vn_deallocate()…
khngAuthorUnsubmitted
Not Done
Inline Actions

The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the policy modules in base so far are not taking f_cred into consideration at all).

khng: The fp->f_cred was there to for mac_vnode_check_write's policy checking (although all the…
khngAuthorUnsubmitted
Done
Inline Actions

A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base policy module, it is possibly used by outside world to implement capability semantic.

khng: A mental note to myself: Though mac_vnode_check_write()'s file_cred is not inspected by in base…
break; break;
default: default:
panic("vn_fspacectl: unknown cmd %d", cmd); panic("vn_fspacectl: unknown cmd %d", cmd);
} }
return (error); return (error);
} }
▲ Show 20 Lines • Show All 101 Lines • Show Last 20 Lines
View Options

sys/dev/hyperv/netvsc/ndis.h

Loading...
View Options

sys/net/rndis.h

Loading...