* [PATCH net-next 0/2] net: dsa: bcm_sf2: GPHY power down
From: Florian Fainelli @ 2015-02-05 19:40 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
Hi David,
This patch series implement GPHY power up and down in the SF2 switch
driver in order to conserve power whenever possible (e.g: port is brought
down or unused during Wake-on-LAN).
Florian Fainelli (2):
net: dsa: bcm_sf2: move GPHY enabling to its own function
net: dsa: bcm_sf2: implement GPHY power down
drivers/net/dsa/bcm_sf2.c | 64 ++++++++++++++++++++++++++++++++++--------
drivers/net/dsa/bcm_sf2_regs.h | 4 +++
2 files changed, 56 insertions(+), 12 deletions(-)
--
2.1.0
^ permalink raw reply
* [PATCH net-next 1/2] net: dsa: bcm_sf2: move GPHY enabling to its own function
From: Florian Fainelli @ 2015-02-05 19:40 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1423165242-6042-1-git-send-email-f.fainelli@gmail.com>
Move the code that touches the single GPHY register from
bcm_sf2_sw_resume() to a separate function since we will have to
enable/disable the GPHY from different locations, and we want the code
to be self-contained.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 09f6b3cc1f66..45c0e2b97f5f 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -233,6 +233,24 @@ static void bcm_sf2_eee_enable_set(struct dsa_switch *ds, int port, bool enable)
core_writel(priv, reg, CORE_EEE_EN_CTRL);
}
+static void bcm_sf2_gphy_enable_set(struct dsa_switch *ds, bool enable)
+{
+ struct bcm_sf2_priv *priv = ds_to_priv(ds);
+ u32 reg;
+
+ if (!enable)
+ return;
+
+ reg = reg_readl(priv, REG_SPHY_CNTRL);
+ reg |= PHY_RESET;
+ reg &= ~(EXT_PWR_DOWN | IDDQ_BIAS);
+ reg_writel(priv, reg, REG_SPHY_CNTRL);
+ udelay(21);
+ reg = reg_readl(priv, REG_SPHY_CNTRL);
+ reg &= ~PHY_RESET;
+ reg_writel(priv, reg, REG_SPHY_CNTRL);
+}
+
static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
struct phy_device *phy)
{
@@ -771,7 +789,6 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
{
struct bcm_sf2_priv *priv = ds_to_priv(ds);
unsigned int port;
- u32 reg;
int ret;
ret = bcm_sf2_sw_rst(priv);
@@ -780,17 +797,8 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
return ret;
}
- /* Reinitialize the single GPHY */
- if (priv->hw_params.num_gphy == 1) {
- reg = reg_readl(priv, REG_SPHY_CNTRL);
- reg |= PHY_RESET;
- reg &= ~(EXT_PWR_DOWN | IDDQ_BIAS);
- reg_writel(priv, reg, REG_SPHY_CNTRL);
- udelay(21);
- reg = reg_readl(priv, REG_SPHY_CNTRL);
- reg &= ~PHY_RESET;
- reg_writel(priv, reg, REG_SPHY_CNTRL);
- }
+ if (priv->hw_params.num_gphy == 1)
+ bcm_sf2_gphy_enable_set(ds, true);
for (port = 0; port < DSA_MAX_PORTS; port++) {
if ((1 << port) & ds->phys_port_mask)
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 2/2] net: dsa: bcm_sf2: implement GPHY power down
From: Florian Fainelli @ 2015-02-05 19:40 UTC (permalink / raw)
To: netdev; +Cc: davem, Florian Fainelli
In-Reply-To: <1423165242-6042-1-git-send-email-f.fainelli@gmail.com>
Implement the power on/off recommended procedure for the Single GPHY we
have on our Starfighter 2 switch. In order to make sure we get proper
LED link/activity signaling during suspend, switch the link indication
from the Switch/MAC to the PHY.
Finally, since the GPHY needs to be reset to be put in low power mode,
we will loose any context applied to it: workarounds, EEE etc.. so we
need to call phy_init_hw() to get our fixups re-applied successfully.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 50 ++++++++++++++++++++++++++++++++++--------
drivers/net/dsa/bcm_sf2_regs.h | 4 ++++
2 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 45c0e2b97f5f..4daffb284931 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -238,17 +238,28 @@ static void bcm_sf2_gphy_enable_set(struct dsa_switch *ds, bool enable)
struct bcm_sf2_priv *priv = ds_to_priv(ds);
u32 reg;
- if (!enable)
- return;
-
- reg = reg_readl(priv, REG_SPHY_CNTRL);
- reg |= PHY_RESET;
- reg &= ~(EXT_PWR_DOWN | IDDQ_BIAS);
- reg_writel(priv, reg, REG_SPHY_CNTRL);
- udelay(21);
reg = reg_readl(priv, REG_SPHY_CNTRL);
- reg &= ~PHY_RESET;
+ if (enable) {
+ reg |= PHY_RESET;
+ reg &= ~(EXT_PWR_DOWN | IDDQ_BIAS | CK25_DIS);
+ reg_writel(priv, reg, REG_SPHY_CNTRL);
+ udelay(21);
+ reg = reg_readl(priv, REG_SPHY_CNTRL);
+ reg &= ~PHY_RESET;
+ } else {
+ reg |= EXT_PWR_DOWN | IDDQ_BIAS | PHY_RESET;
+ reg_writel(priv, reg, REG_SPHY_CNTRL);
+ mdelay(1);
+ reg |= CK25_DIS;
+ }
reg_writel(priv, reg, REG_SPHY_CNTRL);
+
+ /* Use PHY-driven LED signaling */
+ if (!enable) {
+ reg = reg_readl(priv, REG_LED_CNTRL(0));
+ reg |= SPDLNK_SRC_SEL;
+ reg_writel(priv, reg, REG_LED_CNTRL(0));
+ }
}
static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
@@ -266,6 +277,24 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
/* Clear the Rx and Tx disable bits and set to no spanning tree */
core_writel(priv, 0, CORE_G_PCTL_PORT(port));
+ /* Re-enable the GPHY and re-apply workarounds */
+ if (port == 0 && priv->hw_params.num_gphy == 1) {
+ bcm_sf2_gphy_enable_set(ds, true);
+ if (phy) {
+ /* if phy_stop() has been called before, phy
+ * will be in halted state, and phy_start()
+ * will call resume.
+ *
+ * the resume path does not configure back
+ * autoneg settings, and since we hard reset
+ * the phy manually here, we need to reset the
+ * state machine also.
+ */
+ phy->state = PHY_READY;
+ phy_init_hw(phy);
+ }
+ }
+
/* Enable port 7 interrupts to get notified */
if (port == 7)
intrl2_1_mask_clear(priv, P_IRQ_MASK(P7_IRQ_OFF));
@@ -299,6 +328,9 @@ static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
intrl2_1_writel(priv, P_IRQ_MASK(P7_IRQ_OFF), INTRL2_CPU_CLEAR);
}
+ if (port == 0 && priv->hw_params.num_gphy == 1)
+ bcm_sf2_gphy_enable_set(ds, false);
+
if (dsa_is_cpu_port(ds, port))
off = CORE_IMP_CTL;
else
diff --git a/drivers/net/dsa/bcm_sf2_regs.h b/drivers/net/dsa/bcm_sf2_regs.h
index 1bb49cb699ab..cabdfa5e217a 100644
--- a/drivers/net/dsa/bcm_sf2_regs.h
+++ b/drivers/net/dsa/bcm_sf2_regs.h
@@ -61,6 +61,10 @@
#define LPI_COUNT_SHIFT 9
#define LPI_COUNT_MASK 0x3F
+#define REG_LED_CNTRL_BASE 0x90
+#define REG_LED_CNTRL(x) (REG_LED_CNTRL_BASE + (x) * 4)
+#define SPDLNK_SRC_SEL (1 << 24)
+
/* Register set relative to 'INTRL2_0' and 'INTRL2_1' */
#define INTRL2_CPU_STATUS 0x00
#define INTRL2_CPU_SET 0x04
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] dsa: correctly determine the number of switches in a system
From: Florian Fainelli @ 2015-02-05 19:41 UTC (permalink / raw)
To: Tobias Waldekranz, netdev
In-Reply-To: <20150205135409.GB29383@gmail.com>
On 05/02/15 05:54, Tobias Waldekranz wrote:
> The number of connected switches was sourced from the number of
> children to the DSA node, change it to the number of available
> children, skipping any disabled switches.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Fixes: 5e95329b701c4 ("dsa: add device tree bindings to register DSA
switches")
> ---
> net/dsa/dsa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> index 3731714..2173402 100644
> --- a/net/dsa/dsa.c
> +++ b/net/dsa/dsa.c
> @@ -603,7 +603,7 @@ static int dsa_of_probe(struct platform_device *pdev)
>
> pdev->dev.platform_data = pd;
> pd->netdev = ðernet_dev->dev;
> - pd->nr_chips = of_get_child_count(np);
> + pd->nr_chips = of_get_available_child_count(np);
> if (pd->nr_chips > DSA_MAX_SWITCHES)
> pd->nr_chips = DSA_MAX_SWITCHES;
>
>
--
Florian
^ permalink raw reply
* Re: [ovs-dev] [RFC: add openvswitch actions using BPF 1/2] BPF: add a new BPF program type BPF_PROG_TYPE_OPENVSWITCH
From: Andy Zhou @ 2015-02-05 19:47 UTC (permalink / raw)
To: Thomas Graf; +Cc: dev@openvswitch.com, netdev@vger.kernel.org
In-Reply-To: <20150205144828.GF5546@pox.localdomain>
On Thu, Feb 5, 2015 at 6:48 AM, Thomas Graf <tgraf@noironetworks.com> wrote:
> On 02/04/15 at 02:48pm, Andy Zhou wrote:
>> struct bpf_verifier_ops {
>> /* return eBPF function prototype for verification */
>> - const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
>> + const struct bpf_func_proto *(*get_func_proto)(int func_id);
>
> This change should maybe go in a separate commit.
Agreed. Ideally, each program type should have its own func_id space. Current
registration does not allow this. Should this be added?
In the interest of reusing common helper functions, should we also consider
partition the func_id space into common ones and program type specific ones?
For example, the prink like function can be specified the common func_id space.
>
>> +static const struct bpf_func_proto *ovs_func_proto(int func_id)
>> +{
>> + switch (func_id) {
>> + case OVS_BPF_FUNC_output:
>> + return &bpf_helper_output_proto;
>> + default:
>> + return NULL;
>> + }
>> +}
>
> You'd still want to use the map helpers so it seems like we should
> change the bpf verified to verify against both a global and type
> specific list unless we want to add all the map helpers to
> ovs_func_proto as well.
It is not clear what OVS can benefit from the map helpers. Map provides
a fixed sized array. OVS data structure, such as flow, are more
dynamic and non-contiguous in memory.
Is extending BPF to allow for passing those dynamic objects a reasonable
direction?
>
>> +static bool test_is_valid_access(int off, int size, enum bpf_access_type type)
>> +{
>> + const struct bpf_context_access *access;
>> +
>> + if (off < 0 || off >= ARRAY_SIZE(bpf_ctx_access))
>> + return false;
>> +
>> + access = &bpf_ctx_access[off];
>> + if (access->size == size && (access->type & type))
>> + return true;
>> +
>> + return false;
>> +}
>
> OK. I see why you kept ctxt simple at first ;-)
^ permalink raw reply
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Dave Taht @ 2015-02-05 19:50 UTC (permalink / raw)
To: Michal Kazior
Cc: Eric Dumazet, Neal Cardwell, Eyal Perry, netdev@vger.kernel.org,
linux-wireless
In-Reply-To: <CA+BoTQmcShK0U_cXvEOLY_8y7LH8x3taTgjcyMzv0MLVn4UtCA@mail.gmail.com>
On Fri, Feb 6, 2015 at 2:44 AM, Michal Kazior <michal.kazior@tieto.com> wrote:
>
> On 5 February 2015 at 14:19, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Thu, 2015-02-05 at 04:57 -0800, Eric Dumazet wrote:
> >
> >> The intention is to control the queues to the following :
> >>
> >> 1 ms of buffering, but limited to a configurable value.
> >>
> >> On a 40Gbps flow, 1ms represents 5 MB, which is insane.
> >>
> >> We do not want to queue 5 MB of traffic, this would destroy latencies
> >> for all concurrent flows. (Or would require having fq_codel or fq as
> >> packet schedulers, instead of default pfifo_fast)
> >>
> >> This is why having 1.5 ms delay between the transmit and TX completion
> >> is a problem in your case.
>
> I do get your point. But 1.5ms is really tough on Wi-Fi.
>
> Just look at this:
>
> ; ping 192.168.1.2 -c 3
> PING 192.168.1.2 (192.168.1.2) 56(84) bytes of data.
> 64 bytes from 192.168.1.2: icmp_seq=1 ttl=64 time=1.83 ms
> 64 bytes from 192.168.1.2: icmp_seq=2 ttl=64 time=2.02 ms
> 64 bytes from 192.168.1.2: icmp_seq=3 ttl=64 time=1.98 ms
>
> ; ping 192.168.1.2 -c 3 -Q 224
> PING 192.168.1.2 (192.168.1.2) 56(84) bytes of data.
> 64 bytes from 192.168.1.2: icmp_seq=1 ttl=64 time=0.939 ms
> 64 bytes from 192.168.1.2: icmp_seq=2 ttl=64 time=0.906 ms
> 64 bytes from 192.168.1.2: icmp_seq=3 ttl=64 time=0.946 ms
>
> This was run with no load so batching code in the driver itself should
> have no measurable effect. The channel was near-ideal: low noise
> floor, cabled rf, no other traffic.
>
> The lower latency ping is when 802.11 QoS Voice Access Category is
> used. I also get 400mbps instead of 250mbps in this case with 5 flows
> (net/master).
>
The VO queue is now nearly useless in a real world environment. Whlle
it does grab the media mildly faster in some cases, on a good day with
no other competing APs, it cannot aggregate packets, and wastes TXOPS.
It is far saner to aim for better aggregate (use the VI queue if you
must try to get better media acquisition).
It is disabled in multiple products I know of.
And I really, really, really wish, that just once during this thread,
someone had bothered to try running a test
at a real world MCS rate - say MCS1, or MCS4, and measured the latency
under load of that...
or tried talking to two or more stations at the same time.
Instead of trying for 1.5Gbits in a faraday cage.
>
> Dealing with black box firmware blobs is a pain.
>
+10
>
>
> > Note that TCP stack could detect when this happens, *if* ACK where
> > delivered before the TX completions, or when TX completion happens,
> > we could detect that the clone of the freed packet was freed.
> >
> > In my test, when I did "ethtool -C eth0 tx-usecs 1024 tx-frames 64", and
> > disabling GSO, TCP stack sends a bunch of packets (a bit less than 64),
> > blocks on tcp_limit_output_bytes.
> >
> > Then we receive 2 stretch ACKS after ~50 usec.
> >
> > TCP stack tries to push again some packets but blocks on
> > tcp_limit_output_bytes again.
> >
> > 1ms later, TX completion happens, tcp_wfree() is called, and TCP stack
> > push following ~60 packets.
> >
> >
> > TCP could eventually dynamically adjust the tcp_limit_output_bytes,
> > using a per flow dynamic value, but I would rather not add a kludge in
> > TCP stack only to deal with a possible bug in ath10k driver.
> >
> > niu has a similar issue and simply had to call skb_orphan() :
> >
> > drivers/net/ethernet/sun/niu.c:6669: skb_orphan(skb);
>
> Ok. I tried calling skb_orphan() right after I submit each Tx frame
> (similar to niu which does this in start_xmit):
>
> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
> @@ -564,6 +564,8 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct
> sk_buff *msdu)
> if (res)
> goto err_unmap_msdu;
>
> + skb_orphan(msdu);
> +
> return 0;
>
> err_unmap_msdu:
>
>
> Now, with {net/master + ath10k GRO + the above} I get 620mbps on a
> single flow (even better then before). Wow.
>
> Does this look ok/safe as a solution to you?
>
>
> Michał
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Dave Täht
thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
^ permalink raw reply
* Re: [PATCH] af_packet: don't pass empty blocks for PACKET_V3
From: Willem de Bruijn @ 2015-02-05 20:01 UTC (permalink / raw)
To: Alexander Drozdov
Cc: David S. Miller, Daniel Borkmann, Eric Dumazet, Al Viro,
Michael S. Tsirkin, Network Development, linux-kernel
In-Reply-To: <1423115891-3578-1-git-send-email-al.drozdov@gmail.com>
On Wed, Feb 4, 2015 at 9:58 PM, Alexander Drozdov <al.drozdov@gmail.com> wrote:
> Don't close an empty block on timeout. Its meaningless to
> pass it to the user. Moreover, passing empty blocks wastes
> CPU & buffer space increasing probability of packets
> dropping on small timeouts.
>
> Side effect of this patch is indefinite user-space wait
> in poll on idle links. But, I believe its better to set
> timeout for poll(2) when needed than to get empty blocks
> every millisecond when not needed.
This change would break existing applications that have come
to depend on the periodic signal.
I don't disagree with the argument that the data ready signal
should be sent only when a block is full or a timer expires and
at least some data is waiting, but that is moot at this point.
>
> Signed-off-by: Alexander Drozdov <al.drozdov@gmail.com>
> ---
> net/packet/af_packet.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 9cfe2e1..9a2f70a 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -698,6 +698,10 @@ static void prb_retire_rx_blk_timer_expired(unsigned long data)
>
> if (pkc->last_kactive_blk_num == pkc->kactive_blk_num) {
> if (!frozen) {
> + if (!BLOCK_NUM_PKTS(pbd)) {
> + /* An empty block. Just refresh the timer. */
> + goto refresh_timer;
> + }
> prb_retire_current_block(pkc, po, TP_STATUS_BLK_TMO);
> if (!prb_dispatch_next_block(pkc, po))
> goto refresh_timer;
> @@ -798,7 +802,11 @@ static void prb_close_block(struct tpacket_kbdq_core *pkc1,
> h1->ts_last_pkt.ts_sec = last_pkt->tp_sec;
> h1->ts_last_pkt.ts_nsec = last_pkt->tp_nsec;
> } else {
> - /* Ok, we tmo'd - so get the current time */
> + /* Ok, we tmo'd - so get the current time.
> + *
> + * It shouldn't really happen as we don't close empty
> + * blocks. See prb_retire_rx_blk_timer_expired().
> + */
> struct timespec ts;
> getnstimeofday(&ts);
> h1->ts_last_pkt.ts_sec = ts.tv_sec;
> --
> 1.9.1
>
^ permalink raw reply
* Re: [ANNOUNCE] netdev0.1 netfilter BoF
From: Patrick McHardy @ 2015-02-05 20:20 UTC (permalink / raw)
To: Josh Hunt, netdev, netfilter-devel
Cc: Pablo Neira Ayuso, Bohman, Pete, Jozsef Kadlecsik
In-Reply-To: <54D2B6B2.8070004@akamai.com>
Am 5. Februar 2015 01:17:54 MEZ, schrieb Josh Hunt <johunt@akamai.com>:
>We will be leading a BoF discussing netfilter development and issues in
>
>iptables, ipset, and nftables at netdev0.1 in Ottawa coming up in a few
>
>weeks.
>
>Our timeslot is currently scheduled for Sunday, Feb 15 @ 9am, but
>please
>refer to https://www.netdev01.org/schedule in case there are any
>schedule changes.
>
>The topics we intend to cover will be based around Akamai's usage,
>deployment, and the issues we've come across.
>
>Some examples of those topics are:
> * The need for supported interfaces from netfilter components.
> * The handling of large (1 million to 25 million entry) sets, both
>with ipsets and nft sets.
> * Limitations in existing iptables functionality.
> * For ex, issues with hashlimit implementation and proposals to
>extend it.
>
>We'll also look at the current standing of nftables and discuss some
>basic performance as it pertains to our environment, as well as,
>backwards compatibility, and possible new features in nftables.
>
>We hope to bring together interested stakeholders and interested
>parties
>to discuss the above along with any other related netfilter topics you
>would like to discuss. The BoF is scheduled for two hours.
>
>If you have items you'd like to add to the agenda and/or present,
>please
>contact me.
I'll be there.
Cheers,
Patrick
>
>Thanks
>Josh & Pete
^ permalink raw reply
* [PATCH] rds: Make rds_message_copy_from_user() return 0 on success.
From: Sowmini Varadhan @ 2015-02-05 20:20 UTC (permalink / raw)
To: chien.yen, davem; +Cc: rds-devel, netdev, linux-kernel, viro
Commit c310e72c8992 ("rds: switch ->inc_copy_to_user() to passing iov_iter")
breaks rds_message_copy_from_user() semantics on success, and causes it
to return nbytes copied, when it should return 0. This commit fixes that bug.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
net/rds/message.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/rds/message.c b/net/rds/message.c
index 5a21e6f..756c737 100644
--- a/net/rds/message.c
+++ b/net/rds/message.c
@@ -266,7 +266,7 @@ struct rds_message *rds_message_map_pages(unsigned long *page_addrs, unsigned in
int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from)
{
- unsigned long to_copy;
+ unsigned long to_copy, nbytes;
unsigned long sg_off;
struct scatterlist *sg;
int ret = 0;
@@ -293,9 +293,9 @@ int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from)
sg->length - sg_off);
rds_stats_add(s_copy_from_user, to_copy);
- ret = copy_page_from_iter(sg_page(sg), sg->offset + sg_off,
- to_copy, from);
- if (ret != to_copy)
+ nbytes = copy_page_from_iter(sg_page(sg), sg->offset + sg_off,
+ to_copy, from);
+ if (nbytes != to_copy)
return -EFAULT;
sg_off += to_copy;
--
1.7.1
^ permalink raw reply related
* Ethernet: how to disable gigabit support
From: Pavel Machek @ 2015-02-05 20:25 UTC (permalink / raw)
To: kernel list, netdev
Hi!
This happened on more than one project: there's gigabit-capable chip,
but the connector is not designed for gigabit speed.
I'd like to have speed autonegotiation, but not offer gigabit (as it
will not work).
Is there way to do that without hacking the kernel? Should mii-tool do
that?
Thanks,
Pavel
root@ib8315:~# mii-tool.net-tools -A 100baseTx,10baseT eth1
restarting autonegotiation...
root@ib8315:~# [32432.443883] libphy: mdio@e0024520:01 - Link is Down
root@ib8315:~# [32435.447863] libphy: mdio@e0024520:01 - Link is Up - 1000/Full
root@ib8315:~# mii-tool.net-tools -v eth1
eth1: negotiated 1000baseT-FD flow-control, link ok
product info: vendor 00:01:c1, model 3 rev 1
basic mode: autonegotiation enabled
basic status: autonegotiation complete, link ok
capabilities: 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
advertising: 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD
link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD
10baseT-FD 10baseT-HD flow-control
root@ib8315:~# mii-tool.net-tools -V
$Id: mii-tool.c,v 1.9 2006/09/27 20:59:18 ecki Exp $
(Author: David Hinds based on Donald Becker's mii-diag)
net-tools 1.60
root@ib8315:~#
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [ovs-dev] [RFC: add openvswitch actions using BPF 1/2] BPF: add a new BPF program type BPF_PROG_TYPE_OPENVSWITCH
From: Alexei Starovoitov @ 2015-02-05 20:30 UTC (permalink / raw)
To: Andy Zhou; +Cc: Thomas Graf, dev@openvswitch.com, netdev@vger.kernel.org
On Thu, Feb 5, 2015 at 11:47 AM, Andy Zhou <azhou@nicira.com> wrote:
> On Thu, Feb 5, 2015 at 6:48 AM, Thomas Graf <tgraf@noironetworks.com> wrote:
>> On 02/04/15 at 02:48pm, Andy Zhou wrote:
>>> struct bpf_verifier_ops {
>>> /* return eBPF function prototype for verification */
>>> - const struct bpf_func_proto *(*get_func_proto)(enum bpf_func_id func_id);
>>> + const struct bpf_func_proto *(*get_func_proto)(int func_id);
>>
>> This change should maybe go in a separate commit.
> Agreed. Ideally, each program type should have its own func_id space. Current
> registration does not allow this. Should this be added?
>
> In the interest of reusing common helper functions, should we also consider
> partition the func_id space into common ones and program type specific ones?
> For example, the prink like function can be specified the common func_id space.
I don't like per-program_type func_ids, since overlapping ids is
a debugging headache and it encourages hiding ids instead of
clearly adding them to uapi's enum bpf_func_id.
per-program type ids also make it impossible to share common helper ids
which is already the case between socket and tracing prog types.
Having one place and one enum for all ids helps to keep
exposed user helpers under control.
>>> +static const struct bpf_func_proto *ovs_func_proto(int func_id)
>>> +{
>>> + switch (func_id) {
>>> + case OVS_BPF_FUNC_output:
>>> + return &bpf_helper_output_proto;
>>> + default:
>>> + return NULL;
>>> + }
>>> +}
>>
>> You'd still want to use the map helpers so it seems like we should
>> change the bpf verified to verify against both a global and type
>> specific list unless we want to add all the map helpers to
>> ovs_func_proto as well.
>
> It is not clear what OVS can benefit from the map helpers. Map provides
> a fixed sized array. OVS data structure, such as flow, are more
> dynamic and non-contiguous in memory.
that's a misunderstanding.
maps can be different types including rhashtable type.
Actually I was waiting for rhashtable to stabilize
before adding it as new map type ;)
There is 'max_entries' limit that must be there for safety reasons,
but it doesn't mean that all entries are always allocated at
init time and never change.
contiguous vs non-contiguous is also internal detail of map
implementation.
^ permalink raw reply
* Re: [RFC: add openvswitch actions using BPF 5/9] bpf: add the first BPF program.
From: Alexei Starovoitov @ 2015-02-05 20:34 UTC (permalink / raw)
To: Andy Zhou
Cc: dev-yBygre7rU0SM8Zsap4Y0gw@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
On Thu, Feb 5, 2015 at 11:16 AM, Andy Zhou <azhou@nicira.com> wrote:
>>> +bpf/ovs-actions.bpf: $(srcdir)/bpf/ovs-actions.c $(DEP_FILES)
>>> + $(AM_V_GEN)clang -DHAVE_CONFIG_H $(BPF_INCLUDES) $(NOSTDINC_FLAGS) \
>>> + $(AM_CFLAGS) $(EXTRA_CFLAGS) -Wno-unused-value -Wno-pointer-sign \
>>> + -O2 -emit-llvm -c $< -o -| $(LLC) -filetype=obj -o $@
>>
>> I assume you will convert this to a Makefile template
> Yes. It needs to be addressed. Not sure if additional changes are
> required now that
> BPF backend is merged into LLVM upstream,
Once llvm 3.7 is released the command line will be shorter,
since single command 'clang --target=bpf file.c' will do the job.
but that's minor.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [ovs-dev] [RFC: add openvswitch actions using BPF 2/2] openvswitch: implements the BPF_PROG action in datapath
From: Alexei Starovoitov @ 2015-02-05 20:49 UTC (permalink / raw)
To: Andy Zhou; +Cc: Thomas Graf, dev@openvswitch.com, netdev@vger.kernel.org
On Thu, Feb 5, 2015 at 11:34 AM, Andy Zhou <azhou@nicira.com> wrote:
>>
>> I think it should be doable with BPF to have the verifier accept if
>> a helper is called with less args than expected, to initialize those
>> to 0. This would allow for helpers to support additional arguments.
> I am not sure it is fundamentally better than V2, v3... On the other hand,
> I agree this looks technically possible.
yes. kinda.
the easiest is just have a flag-like argument in your helper
and as part of the program always pass zero to it initially.
Later when helper function becomes more advanced it will
start interpreting flag argument differently.
pretty much the same concept used everywhere else.
The helper functions need to be thought through
just like any other user interface.
>> It seems very doable to also allow for a BPF prog to be registered
>> upon flow table miss.
> Yes, this is possible, but at the cost of flow set up rate. It may
> still be practical
> with some optimization, such as caching action lists so we don't
> regenerate or download
> the same program.
Loading the program as soon as user space got a notification
on flow miss is definitely not efficient and not scalable.
Whereas having already loaded program to be called when
flow-miss happens makes a lot of sense.
In program-as-action proposal there are potentially N number
of programs for N flow rules (unless the same program
is used as action for all rules)
In program-after-flow-miss proposal, there is 1 program per
datapath or may be per-vport (so packets from different VMs
can be processed by different programs)
^ permalink raw reply
* Payment
From: Finance Department @ 2015-02-05 14:32 UTC (permalink / raw)
Dear Recipient,
You have been awarded the sum of 8,000,000.00 (Eight Million Pounds sterling) with reference number 77100146 by office of the ministry of finance UK.Send us your personal details to deliver your funds.
Gloria Peter
^ permalink raw reply
* Re: [PATCH] af_packet: don't pass empty blocks for PACKET_V3
From: Guy Harris @ 2015-02-05 21:16 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Alexander Drozdov, David S. Miller, Daniel Borkmann, Eric Dumazet,
Al Viro, Michael S. Tsirkin, Network Development, linux-kernel
In-Reply-To: <CA+FuTScA_2W+MHsZdYfoOKS==1XyogmyF_d+qUy66A5MdyO-5Q@mail.gmail.com>
On Feb 5, 2015, at 12:01 PM, Willem de Bruijn <willemb@google.com> wrote:
> On Wed, Feb 4, 2015 at 9:58 PM, Alexander Drozdov <al.drozdov@gmail.com> wrote:
>> Don't close an empty block on timeout. Its meaningless to
>> pass it to the user. Moreover, passing empty blocks wastes
>> CPU & buffer space increasing probability of packets
>> dropping on small timeouts.
>>
>> Side effect of this patch is indefinite user-space wait
>> in poll on idle links. But, I believe its better to set
>> timeout for poll(2) when needed than to get empty blocks
>> every millisecond when not needed.
>
> This change would break existing applications that have come
> to depend on the periodic signal.
>
> I don't disagree with the argument that the data ready signal
> should be sent only when a block is full or a timer expires and
> at least some data is waiting, but that is moot at this point.
For what it's worth, the BPF packet capture mechanism (which really needs a new name, to distinguish itself from the BPF packet filter language and its implementation(s), but I digress) has the same issue - when the timer expires, a wakeup is delivered even if there are no packets to read.
*However*, if there are no packets available, the buffers aren't rotated, so the empty buffer is left around to be filled up with packets, rather than being made the hold buffer.
Given that before the previous TPACKET_V3 change, wakeups were delivered when packets arrived rather than when a block was closed, presumably code using TPACKET_V3 was capable of dealing with wakeups being delivered when no new blocks had been made available to userland; could TPACKET_V3 work a bit more like BPF and deliver a wakeup when the timer expires *without* closing the empty block?
^ permalink raw reply
* [PATCH v3] net: openvswitch: Support masked set actions.
From: Jarno Rajahalme @ 2015-02-05 21:40 UTC (permalink / raw)
To: netdev, dev; +Cc: jrajahalme
OVS userspace already probes the openvswitch kernel module for
OVS_ACTION_ATTR_SET_MASKED support. This patch adds the kernel module
implementation of masked set actions.
The existing set action sets many fields at once. When only a subset
of the IP header fields, for example, should be modified, all the IP
fields need to be exact matched so that the other field values can be
copied to the set action. A masked set action allows modification of
an arbitrary subset of the supported header bits without requiring the
rest to be matched.
Masked set action is now supported for all writeable key types, except
for the tunnel key. The set tunnel action is an exception as any
input tunnel info is cleared before action processing starts, so there
is no tunnel info to mask.
The kernel module converts all (non-tunnel) set actions to masked set
actions. This makes action processing more uniform, and results in
less branching and duplicating the action processing code. When
returning actions to userspace, the fully masked set actions are
converted back to normal set actions. We use a kernel internal action
code to be able to tell the userspace provided and converted masked
set actions apart.
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
---
v3: Clean up code and remove flow key transport port checks from
validate_set(), as suggested by Jesse Gross.
include/uapi/linux/openvswitch.h | 22 ++-
net/openvswitch/actions.c | 373 ++++++++++++++++++++++++--------------
net/openvswitch/flow_netlink.c | 161 ++++++++++++----
3 files changed, 383 insertions(+), 173 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 7a8785a..bbd49a0 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -599,6 +599,12 @@ struct ovs_action_hash {
* @OVS_ACTION_ATTR_SET: Replaces the contents of an existing header. The
* single nested %OVS_KEY_ATTR_* attribute specifies a header to modify and its
* value.
+ * @OVS_ACTION_ATTR_SET_MASKED: Replaces the contents of an existing header. A
+ * nested %OVS_KEY_ATTR_* attribute specifies a header to modify, its value,
+ * and a mask. For every bit set in the mask, the corresponding bit value
+ * is copied from the value to the packet header field, rest of the bits are
+ * left unchanged. The non-masked value bits must be passed in as zeroes.
+ * Masking is not supported for the %OVS_KEY_ATTR_TUNNEL attribute.
* @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q header onto the
* packet.
* @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q header off the packet.
@@ -617,6 +623,9 @@ struct ovs_action_hash {
* Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
* fields within a header are modifiable, e.g. the IPv4 protocol and fragment
* type may not be changed.
+ *
+ * @OVS_ACTION_ATTR_SET_TO_MASKED: Kernel internal masked set action translated
+ * from the @OVS_ACTION_ATTR_SET.
*/
enum ovs_action_attr {
@@ -631,8 +640,19 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_HASH, /* struct ovs_action_hash. */
OVS_ACTION_ATTR_PUSH_MPLS, /* struct ovs_action_push_mpls. */
OVS_ACTION_ATTR_POP_MPLS, /* __be16 ethertype. */
+ OVS_ACTION_ATTR_SET_MASKED, /* One nested OVS_KEY_ATTR_* including
+ * data immediately followed by a mask.
+ * The data must be zero for the unmasked
+ * bits. */
+
+ __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
+ * from userspace. */
- __OVS_ACTION_ATTR_MAX
+#ifdef __KERNEL__
+ OVS_ACTION_ATTR_SET_TO_MASKED, /* Kernel module internal masked
+ * set action converted from
+ * OVS_ACTION_ATTR_SET. */
+#endif
};
#define OVS_ACTION_ATTR_MAX (__OVS_ACTION_ATTR_MAX - 1)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index b4cffe6..b491c1c 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -185,10 +185,15 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
return 0;
}
-static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
- const __be32 *mpls_lse)
+/* 'KEY' must not have any bits set outside of the 'MASK' */
+#define MASKED(OLD, KEY, MASK) ((KEY) | ((OLD) & ~(MASK)))
+#define SET_MASKED(OLD, KEY, MASK) ((OLD) = MASKED(OLD, KEY, MASK))
+
+static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const __be32 *mpls_lse, const __be32 *mask)
{
__be32 *stack;
+ __be32 lse;
int err;
err = skb_ensure_writable(skb, skb->mac_len + MPLS_HLEN);
@@ -196,14 +201,16 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *key,
return err;
stack = (__be32 *)skb_mpls_header(skb);
+ lse = MASKED(*stack, *mpls_lse, *mask);
if (skb->ip_summed == CHECKSUM_COMPLETE) {
- __be32 diff[] = { ~(*stack), *mpls_lse };
+ __be32 diff[] = { ~(*stack), lse };
+
skb->csum = ~csum_partial((char *)diff, sizeof(diff),
~skb->csum);
}
- *stack = *mpls_lse;
- key->mpls.top_lse = *mpls_lse;
+ *stack = lse;
+ flow_key->mpls.top_lse = lse;
return 0;
}
@@ -230,23 +237,39 @@ static int push_vlan(struct sk_buff *skb, struct sw_flow_key *key,
ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
}
-static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_ethernet *eth_key)
+/* 'src' is already properly masked. */
+static void ether_addr_copy_masked(u8 *dst_, const u8 *src_, const u8 *mask_)
+{
+ u16 *dst = (u16 *)dst_;
+ const u16 *src = (const u16 *)src_;
+ const u16 *mask = (const u16 *)mask_;
+
+ SET_MASKED(dst[0], src[0], mask[0]);
+ SET_MASKED(dst[1], src[1], mask[1]);
+ SET_MASKED(dst[2], src[2], mask[2]);
+}
+
+static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_ethernet *key,
+ const struct ovs_key_ethernet *mask)
{
int err;
+
err = skb_ensure_writable(skb, ETH_HLEN);
if (unlikely(err))
return err;
skb_postpull_rcsum(skb, eth_hdr(skb), ETH_ALEN * 2);
- ether_addr_copy(eth_hdr(skb)->h_source, eth_key->eth_src);
- ether_addr_copy(eth_hdr(skb)->h_dest, eth_key->eth_dst);
+ ether_addr_copy_masked(eth_hdr(skb)->h_source, key->eth_src,
+ mask->eth_src);
+ ether_addr_copy_masked(eth_hdr(skb)->h_dest, key->eth_dst,
+ mask->eth_dst);
ovs_skb_postpush_rcsum(skb, eth_hdr(skb), ETH_ALEN * 2);
- ether_addr_copy(key->eth.src, eth_key->eth_src);
- ether_addr_copy(key->eth.dst, eth_key->eth_dst);
+ ether_addr_copy(flow_key->eth.src, eth_hdr(skb)->h_source);
+ ether_addr_copy(flow_key->eth.dst, eth_hdr(skb)->h_dest);
return 0;
}
@@ -304,6 +327,15 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 l4_proto,
}
}
+static void mask_ipv6_addr(const __be32 old[4], const __be32 addr[4],
+ const __be32 mask[4], __be32 masked[4])
+{
+ masked[0] = MASKED(old[0], addr[0], mask[0]);
+ masked[1] = MASKED(old[1], addr[1], mask[1]);
+ masked[2] = MASKED(old[2], addr[2], mask[2]);
+ masked[3] = MASKED(old[3], addr[3], mask[3]);
+}
+
static void set_ipv6_addr(struct sk_buff *skb, u8 l4_proto,
__be32 addr[4], const __be32 new_addr[4],
bool recalculate_csum)
@@ -315,29 +347,29 @@ static void set_ipv6_addr(struct sk_buff *skb, u8 l4_proto,
memcpy(addr, new_addr, sizeof(__be32[4]));
}
-static void set_ipv6_tc(struct ipv6hdr *nh, u8 tc)
+static void set_ipv6_fl(struct ipv6hdr *nh, u32 fl, u32 mask)
{
- nh->priority = tc >> 4;
- nh->flow_lbl[0] = (nh->flow_lbl[0] & 0x0F) | ((tc & 0x0F) << 4);
+ /* Bits 21-24 are always unmasked, so this retains their values. */
+ SET_MASKED(nh->flow_lbl[0], (u8)(fl >> 16), (u8)(mask >> 16));
+ SET_MASKED(nh->flow_lbl[1], (u8)(fl >> 8), (u8)(mask >> 8));
+ SET_MASKED(nh->flow_lbl[2], (u8)fl, (u8)mask);
}
-static void set_ipv6_fl(struct ipv6hdr *nh, u32 fl)
+static void set_ip_ttl(struct sk_buff *skb, struct iphdr *nh, u8 new_ttl,
+ u8 mask)
{
- nh->flow_lbl[0] = (nh->flow_lbl[0] & 0xF0) | (fl & 0x000F0000) >> 16;
- nh->flow_lbl[1] = (fl & 0x0000FF00) >> 8;
- nh->flow_lbl[2] = fl & 0x000000FF;
-}
+ new_ttl = MASKED(nh->ttl, new_ttl, mask);
-static void set_ip_ttl(struct sk_buff *skb, struct iphdr *nh, u8 new_ttl)
-{
csum_replace2(&nh->check, htons(nh->ttl << 8), htons(new_ttl << 8));
nh->ttl = new_ttl;
}
-static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_ipv4 *ipv4_key)
+static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_ipv4 *key,
+ const struct ovs_key_ipv4 *mask)
{
struct iphdr *nh;
+ __be32 new_addr;
int err;
err = skb_ensure_writable(skb, skb_network_offset(skb) +
@@ -347,36 +379,49 @@ static int set_ipv4(struct sk_buff *skb, struct sw_flow_key *key,
nh = ip_hdr(skb);
- if (ipv4_key->ipv4_src != nh->saddr) {
- set_ip_addr(skb, nh, &nh->saddr, ipv4_key->ipv4_src);
- key->ipv4.addr.src = ipv4_key->ipv4_src;
- }
+ /* Setting an IP addresses is typically only a side effect of
+ * matching on them in the current userspace implementation, so it
+ * makes sense to check if the value actually changed.
+ */
+ if (mask->ipv4_src) {
+ new_addr = MASKED(nh->saddr, key->ipv4_src, mask->ipv4_src);
- if (ipv4_key->ipv4_dst != nh->daddr) {
- set_ip_addr(skb, nh, &nh->daddr, ipv4_key->ipv4_dst);
- key->ipv4.addr.dst = ipv4_key->ipv4_dst;
+ if (unlikely(new_addr != nh->saddr)) {
+ set_ip_addr(skb, nh, &nh->saddr, new_addr);
+ flow_key->ipv4.addr.src = new_addr;
+ }
}
+ if (mask->ipv4_dst) {
+ new_addr = MASKED(nh->daddr, key->ipv4_dst, mask->ipv4_dst);
- if (ipv4_key->ipv4_tos != nh->tos) {
- ipv4_change_dsfield(nh, 0, ipv4_key->ipv4_tos);
- key->ip.tos = nh->tos;
+ if (unlikely(new_addr != nh->daddr)) {
+ set_ip_addr(skb, nh, &nh->daddr, new_addr);
+ flow_key->ipv4.addr.dst = new_addr;
+ }
}
-
- if (ipv4_key->ipv4_ttl != nh->ttl) {
- set_ip_ttl(skb, nh, ipv4_key->ipv4_ttl);
- key->ip.ttl = ipv4_key->ipv4_ttl;
+ if (mask->ipv4_tos) {
+ ipv4_change_dsfield(nh, ~mask->ipv4_tos, key->ipv4_tos);
+ flow_key->ip.tos = nh->tos;
+ }
+ if (mask->ipv4_ttl) {
+ set_ip_ttl(skb, nh, key->ipv4_ttl, mask->ipv4_ttl);
+ flow_key->ip.ttl = nh->ttl;
}
return 0;
}
-static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_ipv6 *ipv6_key)
+static bool is_ipv6_mask_nonzero(const __be32 addr[4])
+{
+ return !!(addr[0] | addr[1] | addr[2] | addr[3]);
+}
+
+static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_ipv6 *key,
+ const struct ovs_key_ipv6 *mask)
{
struct ipv6hdr *nh;
int err;
- __be32 *saddr;
- __be32 *daddr;
err = skb_ensure_writable(skb, skb_network_offset(skb) +
sizeof(struct ipv6hdr));
@@ -384,71 +429,77 @@ static int set_ipv6(struct sk_buff *skb, struct sw_flow_key *key,
return err;
nh = ipv6_hdr(skb);
- saddr = (__be32 *)&nh->saddr;
- daddr = (__be32 *)&nh->daddr;
-
- if (memcmp(ipv6_key->ipv6_src, saddr, sizeof(ipv6_key->ipv6_src))) {
- set_ipv6_addr(skb, ipv6_key->ipv6_proto, saddr,
- ipv6_key->ipv6_src, true);
- memcpy(&key->ipv6.addr.src, ipv6_key->ipv6_src,
- sizeof(ipv6_key->ipv6_src));
- }
- if (memcmp(ipv6_key->ipv6_dst, daddr, sizeof(ipv6_key->ipv6_dst))) {
+ /* Setting an IP addresses is typically only a side effect of
+ * matching on them in the current userspace implementation, so it
+ * makes sense to check if the value actually changed.
+ */
+ if (is_ipv6_mask_nonzero(mask->ipv6_src)) {
+ __be32 *saddr = (__be32 *)&nh->saddr;
+ __be32 masked[4];
+
+ mask_ipv6_addr(saddr, key->ipv6_src, mask->ipv6_src, masked);
+
+ if (unlikely(memcmp(saddr, masked, sizeof(masked)))) {
+ set_ipv6_addr(skb, key->ipv6_proto, saddr, masked,
+ true);
+ memcpy(&flow_key->ipv6.addr.src, masked,
+ sizeof(flow_key->ipv6.addr.src));
+ }
+ }
+ if (is_ipv6_mask_nonzero(mask->ipv6_dst)) {
unsigned int offset = 0;
int flags = IP6_FH_F_SKIP_RH;
bool recalc_csum = true;
-
- if (ipv6_ext_hdr(nh->nexthdr))
- recalc_csum = ipv6_find_hdr(skb, &offset,
- NEXTHDR_ROUTING, NULL,
- &flags) != NEXTHDR_ROUTING;
-
- set_ipv6_addr(skb, ipv6_key->ipv6_proto, daddr,
- ipv6_key->ipv6_dst, recalc_csum);
- memcpy(&key->ipv6.addr.dst, ipv6_key->ipv6_dst,
- sizeof(ipv6_key->ipv6_dst));
+ __be32 *daddr = (__be32 *)&nh->daddr;
+ __be32 masked[4];
+
+ mask_ipv6_addr(daddr, key->ipv6_dst, mask->ipv6_dst, masked);
+
+ if (unlikely(memcmp(daddr, masked, sizeof(masked)))) {
+ if (ipv6_ext_hdr(nh->nexthdr))
+ recalc_csum = (ipv6_find_hdr(skb, &offset,
+ NEXTHDR_ROUTING,
+ NULL, &flags)
+ != NEXTHDR_ROUTING);
+
+ set_ipv6_addr(skb, key->ipv6_proto, daddr, masked,
+ recalc_csum);
+ memcpy(&flow_key->ipv6.addr.dst, masked,
+ sizeof(flow_key->ipv6.addr.dst));
+ }
+ }
+ if (mask->ipv6_tclass) {
+ ipv6_change_dsfield(nh, ~mask->ipv6_tclass, key->ipv6_tclass);
+ flow_key->ip.tos = ipv6_get_dsfield(nh);
+ }
+ if (mask->ipv6_label) {
+ set_ipv6_fl(nh, ntohl(key->ipv6_label),
+ ntohl(mask->ipv6_label));
+ flow_key->ipv6.label =
+ *(__be32 *)nh & htonl(IPV6_FLOWINFO_FLOWLABEL);
+ }
+ if (mask->ipv6_hlimit) {
+ SET_MASKED(nh->hop_limit, key->ipv6_hlimit, mask->ipv6_hlimit);
+ flow_key->ip.ttl = nh->hop_limit;
}
-
- set_ipv6_tc(nh, ipv6_key->ipv6_tclass);
- key->ip.tos = ipv6_get_dsfield(nh);
-
- set_ipv6_fl(nh, ntohl(ipv6_key->ipv6_label));
- key->ipv6.label = *(__be32 *)nh & htonl(IPV6_FLOWINFO_FLOWLABEL);
-
- nh->hop_limit = ipv6_key->ipv6_hlimit;
- key->ip.ttl = ipv6_key->ipv6_hlimit;
return 0;
}
/* Must follow skb_ensure_writable() since that can move the skb data. */
static void set_tp_port(struct sk_buff *skb, __be16 *port,
- __be16 new_port, __sum16 *check)
+ __be16 new_port, __sum16 *check)
{
inet_proto_csum_replace2(check, skb, *port, new_port, 0);
*port = new_port;
- skb_clear_hash(skb);
-}
-
-static void set_udp_port(struct sk_buff *skb, __be16 *port, __be16 new_port)
-{
- struct udphdr *uh = udp_hdr(skb);
-
- if (uh->check && skb->ip_summed != CHECKSUM_PARTIAL) {
- set_tp_port(skb, port, new_port, &uh->check);
-
- if (!uh->check)
- uh->check = CSUM_MANGLED_0;
- } else {
- *port = new_port;
- skb_clear_hash(skb);
- }
}
-static int set_udp(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_udp *udp_port_key)
+static int set_udp(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_udp *key,
+ const struct ovs_key_udp *mask)
{
struct udphdr *uh;
+ __be16 src, dst;
int err;
err = skb_ensure_writable(skb, skb_transport_offset(skb) +
@@ -457,23 +508,40 @@ static int set_udp(struct sk_buff *skb, struct sw_flow_key *key,
return err;
uh = udp_hdr(skb);
- if (udp_port_key->udp_src != uh->source) {
- set_udp_port(skb, &uh->source, udp_port_key->udp_src);
- key->tp.src = udp_port_key->udp_src;
- }
+ /* Either of the masks is non-zero, so do not bother checking them. */
+ src = MASKED(uh->source, key->udp_src, mask->udp_src);
+ dst = MASKED(uh->dest, key->udp_dst, mask->udp_dst);
- if (udp_port_key->udp_dst != uh->dest) {
- set_udp_port(skb, &uh->dest, udp_port_key->udp_dst);
- key->tp.dst = udp_port_key->udp_dst;
+ if (uh->check && skb->ip_summed != CHECKSUM_PARTIAL) {
+ if (likely(src != uh->source)) {
+ set_tp_port(skb, &uh->source, src, &uh->check);
+ flow_key->tp.src = src;
+ }
+ if (likely(dst != uh->dest)) {
+ set_tp_port(skb, &uh->dest, dst, &uh->check);
+ flow_key->tp.dst = dst;
+ }
+
+ if (unlikely(!uh->check))
+ uh->check = CSUM_MANGLED_0;
+ } else {
+ uh->source = src;
+ uh->dest = dst;
+ flow_key->tp.src = src;
+ flow_key->tp.dst = dst;
}
+ skb_clear_hash(skb);
+
return 0;
}
-static int set_tcp(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_tcp *tcp_port_key)
+static int set_tcp(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_tcp *key,
+ const struct ovs_key_tcp *mask)
{
struct tcphdr *th;
+ __be16 src, dst;
int err;
err = skb_ensure_writable(skb, skb_transport_offset(skb) +
@@ -482,50 +550,49 @@ static int set_tcp(struct sk_buff *skb, struct sw_flow_key *key,
return err;
th = tcp_hdr(skb);
- if (tcp_port_key->tcp_src != th->source) {
- set_tp_port(skb, &th->source, tcp_port_key->tcp_src, &th->check);
- key->tp.src = tcp_port_key->tcp_src;
+ src = MASKED(th->source, key->tcp_src, mask->tcp_src);
+ if (likely(src != th->source)) {
+ set_tp_port(skb, &th->source, src, &th->check);
+ flow_key->tp.src = src;
}
-
- if (tcp_port_key->tcp_dst != th->dest) {
- set_tp_port(skb, &th->dest, tcp_port_key->tcp_dst, &th->check);
- key->tp.dst = tcp_port_key->tcp_dst;
+ dst = MASKED(th->dest, key->tcp_dst, mask->tcp_dst);
+ if (likely(dst != th->dest)) {
+ set_tp_port(skb, &th->dest, dst, &th->check);
+ flow_key->tp.dst = dst;
}
+ skb_clear_hash(skb);
return 0;
}
-static int set_sctp(struct sk_buff *skb, struct sw_flow_key *key,
- const struct ovs_key_sctp *sctp_port_key)
+static int set_sctp(struct sk_buff *skb, struct sw_flow_key *flow_key,
+ const struct ovs_key_sctp *key,
+ const struct ovs_key_sctp *mask)
{
+ unsigned int sctphoff = skb_transport_offset(skb);
struct sctphdr *sh;
+ __le32 old_correct_csum, new_csum, old_csum;
int err;
- unsigned int sctphoff = skb_transport_offset(skb);
err = skb_ensure_writable(skb, sctphoff + sizeof(struct sctphdr));
if (unlikely(err))
return err;
sh = sctp_hdr(skb);
- if (sctp_port_key->sctp_src != sh->source ||
- sctp_port_key->sctp_dst != sh->dest) {
- __le32 old_correct_csum, new_csum, old_csum;
+ old_csum = sh->checksum;
+ old_correct_csum = sctp_compute_cksum(skb, sctphoff);
- old_csum = sh->checksum;
- old_correct_csum = sctp_compute_cksum(skb, sctphoff);
+ sh->source = MASKED(sh->source, key->sctp_src, mask->sctp_src);
+ sh->dest = MASKED(sh->dest, key->sctp_dst, mask->sctp_dst);
- sh->source = sctp_port_key->sctp_src;
- sh->dest = sctp_port_key->sctp_dst;
+ new_csum = sctp_compute_cksum(skb, sctphoff);
- new_csum = sctp_compute_cksum(skb, sctphoff);
+ /* Carry any checksum errors through. */
+ sh->checksum = old_csum ^ old_correct_csum ^ new_csum;
- /* Carry any checksum errors through. */
- sh->checksum = old_csum ^ old_correct_csum ^ new_csum;
-
- skb_clear_hash(skb);
- key->tp.src = sctp_port_key->sctp_src;
- key->tp.dst = sctp_port_key->sctp_dst;
- }
+ skb_clear_hash(skb);
+ flow_key->tp.src = sh->source;
+ flow_key->tp.dst = sh->dest;
return 0;
}
@@ -653,52 +720,77 @@ static void execute_hash(struct sk_buff *skb, struct sw_flow_key *key,
key->ovs_flow_hash = hash;
}
-static int execute_set_action(struct sk_buff *skb, struct sw_flow_key *key,
- const struct nlattr *nested_attr)
+static int execute_set_action(struct sk_buff *skb,
+ struct sw_flow_key *flow_key,
+ const struct nlattr *a)
+{
+ /* Only tunnel set execution is supported without a mask. */
+ if (nla_type(a) == OVS_KEY_ATTR_TUNNEL_INFO) {
+ OVS_CB(skb)->egress_tun_info = nla_data(a);
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+/* Mask is at the midpoint of the data. */
+#define get_mask(a, type) ((const type)nla_data(a) + 1)
+
+static int execute_masked_set_action(struct sk_buff *skb,
+ struct sw_flow_key *flow_key,
+ const struct nlattr *a)
{
int err = 0;
- switch (nla_type(nested_attr)) {
+ switch (nla_type(a)) {
case OVS_KEY_ATTR_PRIORITY:
- skb->priority = nla_get_u32(nested_attr);
- key->phy.priority = skb->priority;
+ SET_MASKED(skb->priority, nla_get_u32(a), *get_mask(a, u32 *));
+ flow_key->phy.priority = skb->priority;
break;
case OVS_KEY_ATTR_SKB_MARK:
- skb->mark = nla_get_u32(nested_attr);
- key->phy.skb_mark = skb->mark;
+ SET_MASKED(skb->mark, nla_get_u32(a), *get_mask(a, u32 *));
+ flow_key->phy.skb_mark = skb->mark;
break;
case OVS_KEY_ATTR_TUNNEL_INFO:
- OVS_CB(skb)->egress_tun_info = nla_data(nested_attr);
+ /* Masked data not supported for tunnel. */
+ err = -EINVAL;
break;
case OVS_KEY_ATTR_ETHERNET:
- err = set_eth_addr(skb, key, nla_data(nested_attr));
+ err = set_eth_addr(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_ethernet *));
break;
case OVS_KEY_ATTR_IPV4:
- err = set_ipv4(skb, key, nla_data(nested_attr));
+ err = set_ipv4(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_ipv4 *));
break;
case OVS_KEY_ATTR_IPV6:
- err = set_ipv6(skb, key, nla_data(nested_attr));
+ err = set_ipv6(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_ipv6 *));
break;
case OVS_KEY_ATTR_TCP:
- err = set_tcp(skb, key, nla_data(nested_attr));
+ err = set_tcp(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_tcp *));
break;
case OVS_KEY_ATTR_UDP:
- err = set_udp(skb, key, nla_data(nested_attr));
+ err = set_udp(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_udp *));
break;
case OVS_KEY_ATTR_SCTP:
- err = set_sctp(skb, key, nla_data(nested_attr));
+ err = set_sctp(skb, flow_key, nla_data(a),
+ get_mask(a, struct ovs_key_sctp *));
break;
case OVS_KEY_ATTR_MPLS:
- err = set_mpls(skb, key, nla_data(nested_attr));
+ err = set_mpls(skb, flow_key, nla_data(a), get_mask(a,
+ __be32 *));
break;
}
@@ -818,6 +910,11 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
err = execute_set_action(skb, key, nla_data(a));
break;
+ case OVS_ACTION_ATTR_SET_MASKED:
+ case OVS_ACTION_ATTR_SET_TO_MASKED:
+ err = execute_masked_set_action(skb, key, nla_data(a));
+ break;
+
case OVS_ACTION_ATTR_SAMPLE:
err = sample(dp, skb, key, a);
break;
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 8b9a612..993281e 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -1695,16 +1695,6 @@ static int validate_and_copy_sample(const struct nlattr *attr,
return 0;
}
-static int validate_tp_port(const struct sw_flow_key *flow_key,
- __be16 eth_type)
-{
- if ((eth_type == htons(ETH_P_IP) || eth_type == htons(ETH_P_IPV6)) &&
- (flow_key->tp.src || flow_key->tp.dst))
- return 0;
-
- return -EINVAL;
-}
-
void ovs_match_init(struct sw_flow_match *match,
struct sw_flow_key *key,
struct sw_flow_mask *mask)
@@ -1805,23 +1795,45 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
return err;
}
+/* Return false if there are any non-masked bits set.
+ * Mask follows data immediately, before any netlink padding.
+ */
+static bool validate_masked(u8 *data, int len)
+{
+ u8 *mask = data + len;
+
+ while (len--)
+ if (*data++ & ~*mask++)
+ return false;
+
+ return true;
+}
+
static int validate_set(const struct nlattr *a,
const struct sw_flow_key *flow_key,
struct sw_flow_actions **sfa,
- bool *set_tun, __be16 eth_type, bool log)
+ bool *skip_copy, __be16 eth_type, bool masked, bool log)
{
const struct nlattr *ovs_key = nla_data(a);
int key_type = nla_type(ovs_key);
+ size_t key_len;
/* There can be only one key in a action */
if (nla_total_size(nla_len(ovs_key)) != nla_len(a))
return -EINVAL;
+ key_len = nla_len(ovs_key);
+ if (masked)
+ key_len /= 2;
+
if (key_type > OVS_KEY_ATTR_MAX ||
- (ovs_key_lens[key_type].len != nla_len(ovs_key) &&
+ (ovs_key_lens[key_type].len != key_len &&
ovs_key_lens[key_type].len != OVS_ATTR_NESTED))
return -EINVAL;
+ if (masked && !validate_masked(nla_data(ovs_key), key_len))
+ return -EINVAL;
+
switch (key_type) {
const struct ovs_key_ipv4 *ipv4_key;
const struct ovs_key_ipv6 *ipv6_key;
@@ -1836,7 +1848,10 @@ static int validate_set(const struct nlattr *a,
if (eth_p_mpls(eth_type))
return -EINVAL;
- *set_tun = true;
+ if (masked)
+ return -EINVAL; /* Masked tunnel set not supported. */
+
+ *skip_copy = true;
err = validate_and_copy_set_tun(a, sfa, log);
if (err)
return err;
@@ -1846,48 +1861,66 @@ static int validate_set(const struct nlattr *a,
if (eth_type != htons(ETH_P_IP))
return -EINVAL;
- if (!flow_key->ip.proto)
- return -EINVAL;
-
ipv4_key = nla_data(ovs_key);
- if (ipv4_key->ipv4_proto != flow_key->ip.proto)
- return -EINVAL;
- if (ipv4_key->ipv4_frag != flow_key->ip.frag)
- return -EINVAL;
+ if (masked) {
+ const struct ovs_key_ipv4 *mask = ipv4_key + 1;
+ /* Non-writeable fields. */
+ if (mask->ipv4_proto || mask->ipv4_frag)
+ return -EINVAL;
+ } else {
+ if (ipv4_key->ipv4_proto != flow_key->ip.proto)
+ return -EINVAL;
+
+ if (ipv4_key->ipv4_frag != flow_key->ip.frag)
+ return -EINVAL;
+ }
break;
case OVS_KEY_ATTR_IPV6:
if (eth_type != htons(ETH_P_IPV6))
return -EINVAL;
- if (!flow_key->ip.proto)
- return -EINVAL;
-
ipv6_key = nla_data(ovs_key);
- if (ipv6_key->ipv6_proto != flow_key->ip.proto)
- return -EINVAL;
- if (ipv6_key->ipv6_frag != flow_key->ip.frag)
- return -EINVAL;
+ if (masked) {
+ const struct ovs_key_ipv6 *mask = ipv6_key + 1;
+
+ /* Non-writeable fields. */
+ if (mask->ipv6_proto || mask->ipv6_frag)
+ return -EINVAL;
+
+ /* Invalid bits in the flow label mask? */
+ if (ntohl(mask->ipv6_label) & 0xFFF00000)
+ return -EINVAL;
+ } else {
+ if (ipv6_key->ipv6_proto != flow_key->ip.proto)
+ return -EINVAL;
+ if (ipv6_key->ipv6_frag != flow_key->ip.frag)
+ return -EINVAL;
+ }
if (ntohl(ipv6_key->ipv6_label) & 0xFFF00000)
return -EINVAL;
break;
case OVS_KEY_ATTR_TCP:
- if (flow_key->ip.proto != IPPROTO_TCP)
+ if ((eth_type != htons(ETH_P_IP) &&
+ eth_type != htons(ETH_P_IPV6)) ||
+ flow_key->ip.proto != IPPROTO_TCP)
return -EINVAL;
- return validate_tp_port(flow_key, eth_type);
+ break;
case OVS_KEY_ATTR_UDP:
- if (flow_key->ip.proto != IPPROTO_UDP)
+ if ((eth_type != htons(ETH_P_IP) &&
+ eth_type != htons(ETH_P_IPV6)) ||
+ flow_key->ip.proto != IPPROTO_UDP)
return -EINVAL;
- return validate_tp_port(flow_key, eth_type);
+ break;
case OVS_KEY_ATTR_MPLS:
if (!eth_p_mpls(eth_type))
@@ -1895,15 +1928,45 @@ static int validate_set(const struct nlattr *a,
break;
case OVS_KEY_ATTR_SCTP:
- if (flow_key->ip.proto != IPPROTO_SCTP)
+ if ((eth_type != htons(ETH_P_IP) &&
+ eth_type != htons(ETH_P_IPV6)) ||
+ flow_key->ip.proto != IPPROTO_SCTP)
return -EINVAL;
- return validate_tp_port(flow_key, eth_type);
+ break;
default:
return -EINVAL;
}
+ /* Convert non-masked non-tunnel set actions to masked set actions. */
+ if (!masked && key_type != OVS_KEY_ATTR_TUNNEL) {
+ int start, len = key_len * 2;
+ struct nlattr *at;
+
+ *skip_copy = true;
+
+ start = add_nested_action_start(sfa,
+ OVS_ACTION_ATTR_SET_TO_MASKED,
+ log);
+ if (start < 0)
+ return start;
+
+ at = __add_action(sfa, key_type, NULL, len, log);
+ if (IS_ERR(at))
+ return PTR_ERR(at);
+
+ memcpy(nla_data(at), nla_data(ovs_key), key_len); /* Key. */
+ memset(nla_data(at) + key_len, 0xff, key_len); /* Mask. */
+ /* Clear non-writeable bits from otherwise writeable fields. */
+ if (key_type == OVS_KEY_ATTR_IPV6) {
+ struct ovs_key_ipv6 *mask = nla_data(at) + key_len;
+
+ mask->ipv6_label &= htonl(0x000FFFFF);
+ }
+ add_nested_action_end(*sfa, start);
+ }
+
return 0;
}
@@ -1965,6 +2028,7 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
[OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct ovs_action_push_vlan),
[OVS_ACTION_ATTR_POP_VLAN] = 0,
[OVS_ACTION_ATTR_SET] = (u32)-1,
+ [OVS_ACTION_ATTR_SET_MASKED] = (u32)-1,
[OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
[OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
};
@@ -2060,7 +2124,14 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
case OVS_ACTION_ATTR_SET:
err = validate_set(a, key, sfa,
- &skip_copy, eth_type, log);
+ &skip_copy, eth_type, false, log);
+ if (err)
+ return err;
+ break;
+
+ case OVS_ACTION_ATTR_SET_MASKED:
+ err = validate_set(a, key, sfa,
+ &skip_copy, eth_type, true, log);
if (err)
return err;
break;
@@ -2090,6 +2161,7 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
return 0;
}
+/* 'key' must be the masked key. */
int ovs_nla_copy_actions(const struct nlattr *attr,
const struct sw_flow_key *key,
struct sw_flow_actions **sfa, bool log)
@@ -2177,6 +2249,21 @@ static int set_action_to_attr(const struct nlattr *a, struct sk_buff *skb)
return 0;
}
+static int masked_set_action_to_set_action_attr(const struct nlattr *a,
+ struct sk_buff *skb)
+{
+ const struct nlattr *ovs_key = nla_data(a);
+ size_t key_len = nla_len(ovs_key) / 2;
+
+ /* Revert the conversion we did from a non-masked set action to
+ * masked set action.
+ */
+ if (nla_put(skb, OVS_ACTION_ATTR_SET, nla_len(a) - key_len, ovs_key))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
int ovs_nla_put_actions(const struct nlattr *attr, int len, struct sk_buff *skb)
{
const struct nlattr *a;
@@ -2192,6 +2279,12 @@ int ovs_nla_put_actions(const struct nlattr *attr, int len, struct sk_buff *skb)
return err;
break;
+ case OVS_ACTION_ATTR_SET_TO_MASKED:
+ err = masked_set_action_to_set_action_attr(a, skb);
+ if (err)
+ return err;
+ break;
+
case OVS_ACTION_ATTR_SAMPLE:
err = sample_action_to_attr(a, skb);
if (err)
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] rds: Make rds_message_copy_from_user() return 0 on success.
From: Al Viro @ 2015-02-05 21:58 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: chien.yen, davem, rds-devel, netdev, linux-kernel
In-Reply-To: <20150205202050.GH5811@oracle.com>
On Thu, Feb 05, 2015 at 03:20:50PM -0500, Sowmini Varadhan wrote:
>
> Commit c310e72c8992 ("rds: switch ->inc_copy_to_user() to passing iov_iter")
> breaks rds_message_copy_from_user() semantics on success, and causes it
> to return nbytes copied, when it should return 0. This commit fixes that bug.
*blink*
Commit 083735f4b0 ("rds: switch rds_message_copy_from_user() to iov_iter"),
surely? That's the one that touches copy-from side of things...
And unlike the previous commit (the one you'd mentioned) here the return value
of copy_page_....() ended up passed to caller of its caller. Mea culpa.
So, modulo correction of the commit message,
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
^ permalink raw reply
* Re: [PATCH] rds: Make rds_message_copy_from_user() return 0 on success.
From: Sowmini Varadhan @ 2015-02-05 22:07 UTC (permalink / raw)
To: Al Viro; +Cc: chien.yen, davem, rds-devel, netdev, linux-kernel
In-Reply-To: <20150205215845.GT29656@ZenIV.linux.org.uk>
On (02/05/15 21:58), Al Viro wrote:
> *blink*
>
> Commit 083735f4b0 ("rds: switch rds_message_copy_from_user() to iov_iter"),
> surely? That's the one that touches copy-from side of things...
>
> And unlike the previous commit (the one you'd mentioned) here the return value
> of copy_page_....() ended up passed to caller of its caller. Mea culpa.
>
> So, modulo correction of the commit message,
>
> Acked-by: Al Viro <viro@zeniv.linux.org.uk>
That was the one that git-bisect flagged for me.. I think this one
had the iov_iter_init() in rds_sendmsg() itself? As I recall, this
change was already refactoring rds_message_copy_from_user() (and was the
earliest version that was broken when I tested it)?
should it have been 'put iov_iter into msghdr'? I just wanted
to make sure some innocent user would know how far back to go,
if they wanted to not trip on this one.
--Sowmini
^ permalink raw reply
* [PATCH] net: rds: Remove repeated function names from debug output
From: Rasmus Villemoes @ 2015-02-05 22:17 UTC (permalink / raw)
To: Chien Yen, David S. Miller
Cc: Rasmus Villemoes, rds-devel, netdev, linux-kernel
The macro rdsdebug is defined as
pr_debug("%s(): " fmt, __func__ , ##args)
Hence it doesn't make sense to include the name of the calling
function explicitly in the format string passed to rdsdebug.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
net/rds/ib_send.c | 4 ++--
net/rds/iw_cm.c | 4 ++--
net/rds/iw_send.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/rds/ib_send.c b/net/rds/ib_send.c
index 1dde91e3dc70..bd3825d38abc 100644
--- a/net/rds/ib_send.c
+++ b/net/rds/ib_send.c
@@ -409,7 +409,7 @@ try_again:
posted = IB_GET_POST_CREDITS(oldval);
avail = IB_GET_SEND_CREDITS(oldval);
- rdsdebug("rds_ib_send_grab_credits(%u): credits=%u posted=%u\n",
+ rdsdebug("wanted=%u credits=%u posted=%u\n",
wanted, avail, posted);
/* The last credit must be used to send a credit update. */
@@ -453,7 +453,7 @@ void rds_ib_send_add_credits(struct rds_connection *conn, unsigned int credits)
if (credits == 0)
return;
- rdsdebug("rds_ib_send_add_credits(%u): current=%u%s\n",
+ rdsdebug("credits=%u current=%u%s\n",
credits,
IB_GET_SEND_CREDITS(atomic_read(&ic->i_credits)),
test_bit(RDS_LL_SEND_FULL, &conn->c_flags) ? ", ll_send_full" : "");
diff --git a/net/rds/iw_cm.c b/net/rds/iw_cm.c
index a91e1db62ee6..a6c2bea9f8f9 100644
--- a/net/rds/iw_cm.c
+++ b/net/rds/iw_cm.c
@@ -590,8 +590,8 @@ void rds_iw_conn_shutdown(struct rds_connection *conn)
/* Actually this may happen quite frequently, when
* an outgoing connect raced with an incoming connect.
*/
- rdsdebug("rds_iw_conn_shutdown: failed to disconnect,"
- " cm: %p err %d\n", ic->i_cm_id, err);
+ rdsdebug("failed to disconnect, cm: %p err %d\n",
+ ic->i_cm_id, err);
}
if (ic->i_cm_id->qp) {
diff --git a/net/rds/iw_send.c b/net/rds/iw_send.c
index 9105ea03aec5..13834780a308 100644
--- a/net/rds/iw_send.c
+++ b/net/rds/iw_send.c
@@ -361,7 +361,7 @@ try_again:
posted = IB_GET_POST_CREDITS(oldval);
avail = IB_GET_SEND_CREDITS(oldval);
- rdsdebug("rds_iw_send_grab_credits(%u): credits=%u posted=%u\n",
+ rdsdebug("wanted=%u credits=%u posted=%u\n",
wanted, avail, posted);
/* The last credit must be used to send a credit update. */
@@ -405,7 +405,7 @@ void rds_iw_send_add_credits(struct rds_connection *conn, unsigned int credits)
if (credits == 0)
return;
- rdsdebug("rds_iw_send_add_credits(%u): current=%u%s\n",
+ rdsdebug("credits=%u current=%u%s\n",
credits,
IB_GET_SEND_CREDITS(atomic_read(&ic->i_credits)),
test_bit(RDS_LL_SEND_FULL, &conn->c_flags) ? ", ll_send_full" : "");
--
2.1.3
^ permalink raw reply related
* Re: [PATCH] rds: Make rds_message_copy_from_user() return 0 on success.
From: Al Viro @ 2015-02-05 22:23 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: chien.yen, davem, rds-devel, netdev, linux-kernel
In-Reply-To: <20150205220734.GC28883@oracle.com>
On Thu, Feb 05, 2015 at 05:07:34PM -0500, Sowmini Varadhan wrote:
> That was the one that git-bisect flagged for me.. I think this one
> had the iov_iter_init() in rds_sendmsg() itself?
No, it doesn't - it only touches recvmsg side of things.
> As I recall, this
> change was already refactoring rds_message_copy_from_user() (and was the
> earliest version that was broken when I tested it)?
>
> should it have been 'put iov_iter into msghdr'? I just wanted
> to make sure some innocent user would know how far back to go,
> if they wanted to not trip on this one.
Look at your own patch; it affects only sendmsg() path, as does
"rds: switch rds_message_copy_from_user() to iov_iter". In fact, the code
it fixes had been introduced in that commit. I agree that the bug is
there and I agree with your fix; the only problem I have is your interpretation
of bisect. "rds: switch ->inc_copy_to_user() to passing iov_iter" is
the last one *not* to have that particular breakage.
Seriously, reread both commits and your fix:
"rds: switch ->inc_copy_to_user() to passing iov_iter" - only recvmsg side
"rds: switch rds_message_copy_from_user() to iov_iter" - only sendmsg side
your fix to rds_message_copy_from_user() - only sendmsg side
^ permalink raw reply
* Re: [PATCH net-next] drivers: net: cpsw: make cpsw_ale.c a module to allow re-use on Keystone
From: Murali Karicheri @ 2015-02-05 22:34 UTC (permalink / raw)
To: Tony Lindgren
Cc: Arnd Bergmann, w-kwok2, davem, mugunthanvnm, prabhakar.csengg,
grygorii.strashko, lokeshvutla, mpa, lsorense, netdev,
linux-kernel
In-Reply-To: <20150202164032.GA9418@atomide.com>
On 02/02/2015 11:40 AM, Tony Lindgren wrote:
> * Arnd Bergmann<arnd@arndb.de> [150129 15:51]:
>> On Thursday 29 January 2015 18:15:51 Murali Karicheri wrote:
>>> NetCP on Keystone has cpsw ale function similar to other TI SoCs
>>> and this driver is re-used. To allow both ti cpsw and keystone netcp
>>> to re-use the driver, convert the cpsw ale to a module and configure
>>> it through Kconfig option CONFIG_TI_CPSW_ALE. Currently it is statically
>>> linked to both TI CPSW and NetCP and this causes issues when the above
>>> drivers are built as dynamic modules. This patch addresses this issue
>>>
>>> While at it, fix the Makefile and code to build both netcp_core and
>>> netcp_ethss as dynamic modules. This is needed to support arm allmodconfig.
>>> This also requires exporting of API calls provided by netcp_core so that
>>> both the above can be dynamic modules.
>>>
>>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com>
>>> ---
>>> drivers/net/ethernet/ti/Kconfig | 19 +++++++++++++++++--
>>> drivers/net/ethernet/ti/Makefile | 8 +++++---
>>> drivers/net/ethernet/ti/cpsw_ale.c | 26 ++++++++++++++++++++++++--
>>> drivers/net/ethernet/ti/netcp_core.c | 8 ++++++++
>>> drivers/net/ethernet/ti/netcp_ethss.c | 5 +++++
>>> 5 files changed, 59 insertions(+), 7 deletions(-)
>>
>> I was hoping there would be a way without exporting all those symbols, but
>> I also couldn't come up with a better solution. I'm putting this into the
>> randconfig build test for now, but I'm guessing it's fine.
>
> Probably the best way in the long run is to add a single exported
> function to cpsw-common.c I just added for the MAC address function.
If understand correctly, what you have done is moved the common mac
function and exported the function in cpsw-common.c and called it from
cpsw.c. How is this any different from exporting all common functions
from cpsw_ale.c as is done today? Not sure what you meant by a single
exported function. Are you talking about defining a ale_ops struct of
function ptrs and exporting that instead of individual functions? So
cpsw_ale_common.c
Move all of the common functions here and define them as static.
Defined cpsw_ale_ops and export it.
cpsw.c and netcp_ethss.c calls something like
cpsw_ale_ops.foo();
Murali
>
> Then all the cpsw like drivers can register with that instead of
> having tons of custom exported functions.
>
> But before doing that, we should have a clear idea what all can
> be shared. Murali, maybe you can take a look at that?
>
> Regards,
>
> Tony
--
Murali Karicheri
Linux Kernel, Texas Instruments
^ permalink raw reply
* Re: [PATCH] flowcache: Fix kernel panic in flow_cache_flush_task
From: David Miller @ 2015-02-05 22:39 UTC (permalink / raw)
To: eric.dumazet
Cc: mu, steffen.klassert, herbert, netdev, linux-kernel, torvalds, tc,
jh, fan.du
In-Reply-To: <1423154648.31870.81.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 05 Feb 2015 08:44:08 -0800
> On Thu, 2015-02-05 at 16:36 +0100, Miroslav Urbanek wrote:
>> flow_cache_flush_task references a structure member flow_cache_gc_work
>> where it should reference flow_cache_flush_task instead.
>>
>> Kernel panic occurs on kernels using IPsec during XFRM garbage
>> collection. The garbage collection interval can be shortened using the
>> following sysctl settings:
>>
>> net.ipv4.xfrm4_gc_thresh=4
>> net.ipv6.xfrm6_gc_thresh=4
>>
>> With the default settings, our productions servers crash approximately
>> once a week. With the settings above, they crash immediately.
>>
>> Fixes: ca925cf1534e ("flowcache: Make flow cache name space aware")
>> Reported-by: Tomáš Charvát <tc@excello.cz>
>> Tested-by: Jan Hejl <jh@excello.cz>
>> Signed-off-by: Miroslav Urbanek <mu@miroslavurbanek.com>
>> ---
>
> Nice catch !
>
> Ok, but you do not need to cc so many people/lists
>
> netdev is fine, and we usually CC patch author, just in case.
>
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [net-next 03/16] ixgbe: cleanup sparse errors in new ixgbe_x550.c file
From: David Miller @ 2015-02-05 22:40 UTC (permalink / raw)
To: David.Laight
Cc: jeffrey.t.kirsher, donald.c.skidmore, netdev, nhorman, sassmann,
jogreene
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CADC3FA@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Thu, 5 Feb 2015 09:48:24 +0000
> From: Jeff Kirsher
>> This patch cleans up prototypes that should have been defined
>> as static.
>>
>> Signed-off-by: Don Skidmore <donald.c.skidmore@intel.com>
>> Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 4 +-
>> drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 65 ++++++++++++++-------------
>> 2 files changed, 37 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> index d101b25..1f301ec 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
>> @@ -2451,8 +2451,8 @@ struct ixgbe_hic_read_shadow_ram {
>>
>> struct ixgbe_hic_write_shadow_ram {
>> union ixgbe_hic_hdr2 hdr;
>> - u32 address;
>> - u16 length;
>> + __be32 address;
>> + __be16 length;
>> u16 pad2;
>> u16 data;
>> u16 pad3;
>
> Not related to commit message
Agreed, fix this up.
^ permalink raw reply
* [PATCHv2] rds: Make rds_message_copy_from_user() return 0 on success.
From: Sowmini Varadhan @ 2015-02-05 22:41 UTC (permalink / raw)
To: chien.yen, davem; +Cc: rds-devel, netdev, linux-kernel, sowmini.varadhan, viro
In-Reply-To: <20150205222301.GU29656@ZenIV.linux.org.uk>
Commit 083735f4b01b ("rds: switch rds_message_copy_from_user() to iov_iter")
breaks rds_message_copy_from_user() semantics on success, and causes it
to return nbytes copied, when it should return 0. This commit fixes that bug.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
changes from v1: incorporate Al.Viro comment.
net/rds/message.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/rds/message.c b/net/rds/message.c
index 5a21e6f..756c737 100644
--- a/net/rds/message.c
+++ b/net/rds/message.c
@@ -266,7 +266,7 @@ struct rds_message *rds_message_map_pages(unsigned long *page_addrs, unsigned in
int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from)
{
- unsigned long to_copy;
+ unsigned long to_copy, nbytes;
unsigned long sg_off;
struct scatterlist *sg;
int ret = 0;
@@ -293,9 +293,9 @@ int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from)
sg->length - sg_off);
rds_stats_add(s_copy_from_user, to_copy);
- ret = copy_page_from_iter(sg_page(sg), sg->offset + sg_off,
- to_copy, from);
- if (ret != to_copy)
+ nbytes = copy_page_from_iter(sg_page(sg), sg->offset + sg_off,
+ to_copy, from);
+ if (nbytes != to_copy)
return -EFAULT;
sg_off += to_copy;
--
1.7.1
^ permalink raw reply related
* Re: Ethernet: how to disable gigabit support
From: Florian Fainelli @ 2015-02-05 22:44 UTC (permalink / raw)
To: Pavel Machek, kernel list, netdev
In-Reply-To: <20150205202527.GA7917@amd>
On 05/02/15 12:25, Pavel Machek wrote:
> Hi!
>
> This happened on more than one project: there's gigabit-capable chip,
> but the connector is not designed for gigabit speed.
>
> I'd like to have speed autonegotiation, but not offer gigabit (as it
> will not work).
>
> Is there way to do that without hacking the kernel? Should mii-tool do
> that?
Since you use the PHY library, you should be able to do something like
this in your PHY driver prior to starting the PHY state machine:
phydev->supported &= PHY_BASIC_FEATURES (effectively masking Gigabit
capability)
See drivers/net/ethernet/broadcom/genet/bcmmii.c for instance where this
is done based on the type of interface (RevMII, MII) we are connected to.
>
> Thanks,
> Pavel
>
> root@ib8315:~# mii-tool.net-tools -A 100baseTx,10baseT eth1
> restarting autonegotiation...
> root@ib8315:~# [32432.443883] libphy: mdio@e0024520:01 - Link is Down
>
> root@ib8315:~# [32435.447863] libphy: mdio@e0024520:01 - Link is Up - 1000/Full
>
> root@ib8315:~# mii-tool.net-tools -v eth1
> eth1: negotiated 1000baseT-FD flow-control, link ok
> product info: vendor 00:01:c1, model 3 rev 1
> basic mode: autonegotiation enabled
> basic status: autonegotiation complete, link ok
> capabilities: 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> advertising: 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD
> link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD
> 10baseT-FD 10baseT-HD flow-control
> root@ib8315:~# mii-tool.net-tools -V
> $Id: mii-tool.c,v 1.9 2006/09/27 20:59:18 ecki Exp $
> (Author: David Hinds based on Donald Becker's mii-diag)
> net-tools 1.60
> root@ib8315:~#
>
>
--
Florian
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox