Page MenuHomeFreeBSD

svc_vc.c: Add support for an xp_extpg boolean
Needs ReviewPublic

Authored by rmacklem on Mon, Feb 9, 10:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Feb 11, 5:10 PM
Unknown Object (File)
Wed, Feb 11, 9:44 AM
Unknown Object (File)
Wed, Feb 11, 3:51 AM
Unknown Object (File)
Tue, Feb 10, 1:00 PM
Unknown Object (File)
Tue, Feb 10, 7:15 AM
Unknown Object (File)
Tue, Feb 10, 5:42 AM
Unknown Object (File)
Tue, Feb 10, 3:53 AM
Unknown Object (File)
Tue, Feb 10, 1:04 AM
Subscribers

Details

Reviewers
zlei
Summary

This patch adds a boolean called xp_extpg to the
SVCXPRT structure, which indicates that the NIC
that will be used for RPC messages sent on the
socket can handle M_EXTPG mbufs.

If xp_extpg gets set when the outbound NIC
cannot handle M_EXTPG mbufs, the code in
ip_outout() will call mb_unmapped_to_ext() to
handle the situation.

xp_extpg will be used by the NFS server to
decide if Read replies can be stored in M_EXTPG
mbufs.

Test Plan

Tested by using this patch with the NFS server
patch on both systems that have Mellanox NICs
(which set IFCAP_MEXTPG) and localhost, which
does not set IFCAP_MEXTPG.

Thanks go to Greg Becker <becker.greg@att.net>
for doing the testing using Mellanox NICs.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/rpc/svc_vc.c
321

Oh I'm not an expert on /sys/rpc, really :)

You can use rib_lookup() to ease the code a bit :)

375

The routing table is dynamic. So to make it perfect, and you may want to register the rib ( routing table ) change events via rib_subscribe(), and update xp_extpg accordingly. So that you can benefit it when new nexthop has IFCAP_MEXTPG enabled.

Probably another keen is, emitting IFNET_EVENT_CAPEN event when the enabled capability of an interface is changed, so that the consumers are possible to receive an IFCAP_MEXTPG change and react on, but not instead pulling, which wastes many resources.

sys/rpc/svc_vc.c
321

Maybe. I think I still have to do a
in6_splitscope() call for IPv6, so it
still ends up as a IPv4 vs IPv6 switch
statement.

rib_lookup() also requires NET_EPOCH().
Does that just mean that the call needs
to be wrapped between:

NET_EPOCH_ENTER(et);
rib_looup(..);
NET_EPOCH_EXIT(et);

or does it require more than that.

If I still need to do a in6_splitscope()
call and NET_EPOCH, I'm not sure the code
gets similar, but I will do so, if you think
it is better?

375

xp_extpg is just a hint and nothing breaks if
it is set erroneously. As such, I don't this needs
to worry about routing changes.

Also, NFS servers won't typically have routing
table changes (they tend to be leaf nodes that
aren't changed, at least w.r.t. networking for months
or years).

To be honest, I recall glebius@ suggesting I just
enable use of M_EXTPG always and let ip_output()
call mb_unmapped_to_ext() to deal with it.
--> I just felt that doing the mapping for most

NICs wasn't ideal until most NIC drivers can
handle M_EXTPG mbufs.

I'm not sure what you are referring to w.r.t.
IFNET_EVENT_CAPEN, but all that happens
is that xp_extpg is set when the TCP connection
is made (not on every RPC) and checked once/RPC,
so I don't think overhead is an issue?

Added NET_EPOCH_ENTER()/NET_EPOCH_EXIT() and
cleaned up the code a bit.

I stayed with fib_lookup()/fib6_lookup() since I had
to call in6_splitscope() for IPv6 and the returned
arguments match what fib_lookup() needs.
(For rib_lookup(), the in6_addr would have to be
copied into sin6_addr.)

sys/rpc/svc_vc.c
321

Hmm, it looks like NET_EPOCH is required.
NET_EPOCH_ENTER()/NET_EPOCH_EXIT()
has been added.

I didn't change it to rib_lookup(), since
the code still needs to call in6_splitscope()
and it returns arguments appropriate for
fib6_lookup().

zlei added inline comments.
sys/rpc/svc_vc.c
375

To be honest, I recall glebius@ suggesting I just enable use of M_EXTPG always and let ip_output() call mb_unmapped_to_ext() to deal with it.

Do you have benchmarks for these two setup over an interface without IFCAP_MEXTPG enabled ?

  1. set xprt->xp_extpg = FALSE
  2. set xprt->xp_extpg = TRUE

If the benchmark shows the no noticeable performance degradation, then @glebius 's suggestion is much better and simpler.

375

xp_extpg is just a hint and nothing breaks if it is set erroneously. As such, I don't this needs to worry about routing changes.

Also, NFS servers won't typically have routing table changes (they tend to be leaf nodes that aren't changed, at least w.r.t. networking for months or years).

Indeed.

375

I'm not sure what you are referring to w.r.t. IFNET_EVENT_CAPEN, but all that happens is that xp_extpg is set when the TCP connection is made (not on every RPC) and checked once/RPC, so I don't think overhead is an issue?

Currently there is no such an event IFNET_EVENT_CAPEN exists. If always use M_EXTPG mbuf has noticeable performance degradation over interface without IFCAP_MEXTPG enabled, then this event is valuable.

sys/rpc/svc_vc.c
375

No, I do not have that. I only have very slow
NICs (100Mbps) and they just run at wire speed.

A tester did find a 14% improvement with the
patch (and associated NFS server patch) using
a 100Gbps Mellanox NIC that sets M_EXTPG.

My concern with always enabling it would be
a resource exhaustion for a small system, since
mb_unmapped_to_ext() uses the Sbuf interface
to map the pages. (I don't know if there is a
limit on how much of that can be done?)

I do still have a 256Mbyte i386 at home.
I can test using that in a couple of weeks
to see what happens if you enable M_EXTPG
for it (it has a 100Mbps broadcom NIC).