* Re: [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
From: Maciej Żenczykowski @ 2016-09-27 2:23 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: David S . Miller, netdev@vger.kernel.org, Erik Kline
In-Reply-To: <CAKD1Yr2MHqFMmJyf89pWzopcr07ziVcHqUB+oNWdphKx95nGDg@mail.gmail.com>
> Given that some of this patch checks for == -1, and some of it checks
> for != 0... is it possible that setting the value to something
> unexpected like -3 will cause any issues to the stack? (Other than
> just rendering IPv6 unusable on this interface, which seems like a
> given.)
You shouldn't be able to set it to -3, that's what the extra1 is for...
^ permalink raw reply
* Re: [PATCH v2 3/7] ipv6 addrconf: rtr_solicits == -1 means unlimited
From: Erik Kline @ 2016-09-27 2:25 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: Lorenzo Colitti, David S . Miller, netdev@vger.kernel.org
In-Reply-To: <CANP3RGdqvWsRRW=UFnC=z+1QDSajsfmswjs+BZB8jB+1NkGiqQ@mail.gmail.com>
On 27 September 2016 at 11:23, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>> Given that some of this patch checks for == -1, and some of it checks
>> for != 0... is it possible that setting the value to something
>> unexpected like -3 will cause any issues to the stack? (Other than
>> just rendering IPv6 unusable on this interface, which seems like a
>> given.)
>
> You shouldn't be able to set it to -3, that's what the extra1 is for...
the proc_dointvec_minmax reference with &minus_one means you shouldn't
be able to set it below -1.
^ permalink raw reply
* Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'
From: Maciej Żenczykowski @ 2016-09-27 2:30 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David S . Miller, Linux NetDev, Erik Kline, Lorenzo Colitti
In-Reply-To: <87d7f248-4d95-4253-78ae-6dd110db6330@stressinduktion.org>
> Is seconds granular enough?
The only reason why one would ever want to go into fractions of
seconds would be some sort of unittesting with very low delays.
In any normal environment the max is going to be tens if not hundreds
or thousands of seconds.
Also note that the delay and interval (ie. not max interval) are also
currently exported in seconds, so having more granularity for
max_seconds is kind of pointless.
I have been considering whether I could make proc_dointvec_jiffies
accept floating point input (and output) though... although that seems
a little harder and probably out of scope of this change.
^ permalink raw reply
* [PATCH] i40e: Make struct i40e_stats const
From: Joe Perches @ 2016-09-27 3:17 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: intel-wired-lan, netdev, linux-kernel
Move some data to text
$ size drivers/net/ethernet/intel/i40e/i40e_ethtool.o*
text data bss dec hex filename
25012 0 32 25044 61d4 drivers/net/ethernet/intel/i40e/i40e_ethtool.o.new
22868 2120 32 25020 61bc drivers/net/ethernet/intel/i40e/i40e_ethtool.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 1835186b62c9..d8847a19dc6b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -104,7 +104,7 @@ static const struct i40e_stats i40e_gstrings_misc_stats[] = {
* The PF_STATs are appended to the netdev stats only when ethtool -S
* is queried on the base PF netdev, not on the VMDq or FCoE netdev.
*/
-static struct i40e_stats i40e_gstrings_stats[] = {
+static const struct i40e_stats i40e_gstrings_stats[] = {
I40E_PF_STAT("rx_bytes", stats.eth.rx_bytes),
I40E_PF_STAT("tx_bytes", stats.eth.tx_bytes),
I40E_PF_STAT("rx_unicast", stats.eth.rx_unicast),
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* [PATCH] igb: Realign bad indentation
From: Joe Perches @ 2016-09-27 3:46 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: intel-wired-lan, netdev, linux-kernel
Statements should start on tabstops.
Use a single statement and test instead of multiple tests.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/ethernet/intel/igb/e1000_mac.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_mac.c b/drivers/net/ethernet/intel/igb/e1000_mac.c
index 5010e2232c50..5eff82678f0b 100644
--- a/drivers/net/ethernet/intel/igb/e1000_mac.c
+++ b/drivers/net/ethernet/intel/igb/e1000_mac.c
@@ -792,15 +792,13 @@ static s32 igb_set_default_fc(struct e1000_hw *hw)
* control setting, then the variable hw->fc will
* be initialized based on a value in the EEPROM.
*/
- if (hw->mac.type == e1000_i350) {
+ if (hw->mac.type == e1000_i350)
lan_offset = NVM_82580_LAN_FUNC_OFFSET(hw->bus.func);
- ret_val = hw->nvm.ops.read(hw, NVM_INIT_CONTROL2_REG
- + lan_offset, 1, &nvm_data);
- } else {
- ret_val = hw->nvm.ops.read(hw, NVM_INIT_CONTROL2_REG,
- 1, &nvm_data);
- }
+ else
+ lan_offset = 0;
+ ret_val = hw->nvm.ops.read(hw, NVM_INIT_CONTROL2_REG + lan_offset,
+ 1, &nvm_data);
if (ret_val) {
hw_dbg("NVM Read Error\n");
goto out;
@@ -808,8 +806,7 @@ static s32 igb_set_default_fc(struct e1000_hw *hw)
if ((nvm_data & NVM_WORD0F_PAUSE_MASK) == 0)
hw->fc.requested_mode = e1000_fc_none;
- else if ((nvm_data & NVM_WORD0F_PAUSE_MASK) ==
- NVM_WORD0F_ASM_DIR)
+ else if ((nvm_data & NVM_WORD0F_PAUSE_MASK) == NVM_WORD0F_ASM_DIR)
hw->fc.requested_mode = e1000_fc_tx_pause;
else
hw->fc.requested_mode = e1000_fc_full;
--
2.10.0.rc2.1.g053435c
^ permalink raw reply related
* Re: [PATCH net-next v11 5/6] openvswitch: add layer 3 flow/port support
From: pravin shelar @ 2016-09-27 4:09 UTC (permalink / raw)
To: Jiri Benc; +Cc: ovs dev, Simon Horman, Linux Kernel Network Developers
In-Reply-To: <20160926185306.164bc044@griffin>
On Mon, Sep 26, 2016 at 9:53 AM, Jiri Benc <jbenc@redhat.com> wrote:
> Reviving a very old thread, sorry. Simon handed this over to me, I'm
> preparing v12.
>
> On Fri, 15 Jul 2016 14:07:37 -0700, pravin shelar wrote:
>> I am not sure if you can use only mac_len to detect L3 packet. This
>> does not work with MPLS packets, mac_len is used to account MPLS
>> headers pushed on skb. Therefore in case of a MPLS header on L3
>> packet, mac_len would be non zero and we have to look at either
>> mac_header or some other metadata like is_layer3 flag from key to
>> check for L3 packet.
>
> I went through the relevant code paths and I don't see any problem in
> using mac_len for that. MPLS GSO seems to work correctly. The kernel
> MPLS code expects mac_len to be just the L2 header len, excluding MPLS.
> The same is the case for openvswitch (you're not correct that "mac_len
> is used to account MPLS headers pushed on skb", at least not with the
> current code). In no place I see any problem with mac_len being 0, the
> calculations just nicely work.
>
> What was your concern with that, Pravin?
>
> In another mail in this thread you mentioned skb_mpls_header. That one
> works correctly with mac_len == 0 if mac_header points to the beginning
> of the packet.
>
> You also wrote:
>
>> I was thinking in overall networking stack rather than just ovs
>> datapath. I think we should have consistent method of detecting L3
>> packet. As commented in previous mail it could be achieved using
>> skb-protocol and device type.
>
> Again, mac_len == 0 works correctly and consistently, provided that
> both mac_header and network_header point to the same place. In case of
> a MPLS packet it would be the beginning of MPLS headers.
>
>> > --- a/include/net/mpls.h
>> > +++ b/include/net/mpls.h
>> > @@ -34,6 +34,8 @@ static inline bool eth_p_mpls(__be16 eth_type)
>> > */
>> > static inline unsigned char *skb_mpls_header(struct sk_buff *skb)
>> > {
>> > - return skb_mac_header(skb) + skb->mac_len;
>> > + return skb_mac_header_was_set(skb) ?
>> > + skb_mac_header(skb) + skb->mac_len :
>> > + skb->data;
>> > }
>>
>> This function is also called from GSO layer.
>
> I don't see it used anywhere outside of openvswitch. Not even when
> grepping git history. I may be missing something, though.
>
>> issue is in GSO layer, it
>> does reset mac header and mac length and then calls mpls-gso-handler.
>> So all subsequent check for L3 packet fails.
>> So far we have explored three different ways to detect L3 packet but
>> each has its own issue.
>> 1. skb mac header : GSO can reset mac header.
>> 2. skb mac length : MPLS uses mac_len to account for MPLS header
>> length along with L2 header
>
> It does not appear to be the case. Or at least not anymore.
>
>> 3. skb protocol: ETH_P_TEB is not set for all L2 frames, networking
>> stack is not ready to handle this type for given skb.
>>
>> So none of them works consistently. I think the only option to detect
>> L3 packet reliably (and without adding field to skb) is to use
>> skb-protocol along with ARPHRD_NONE device type. If ARPHRD_NONE type
>> device generates L2 packet it needs to set protocol to ETH_P_TEB. Some
>> networking stack function also needs to be fixed to handle this
>> protocol type, e.g. vlan_get_protocol(), br_dev_queue_push_xmit(),
>> etc.
>
> All of this said, I'm not opposed to using the skb_eth_header_present
> helper and checking the device type, it works. I just want to understand
> whether I missed some problem with mac_len. Seems to make some things
> simpler if we could use mac_len.
>
After commit 48d2ab609b6bb ("net: mpls: Fixups for GSO") MPLS does not
need to use skb mac-len to track the header, so using mac-len test for
L3 packet detection would result in better and cleaner solution.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH v3 2/2] netfilter: Create revision 2 of xt_hashlimit to support higher pps rates
From: Liping Zhang @ 2016-09-27 4:15 UTC (permalink / raw)
To: Vishwanath Pai
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik, johunt,
netfilter-devel, coreteam, netdev, pai.vishwain
In-Reply-To: <20160922164344.GB20423@akamai.com>
Hi Vishwanath,
2016-09-23 0:43 GMT+08:00 Vishwanath Pai <vpai@akamai.com>:
>
> /* Precision saver. */
> -static u32 user2credits(u32 user)
> +static u64 user2credits(u64 user, int revision)
> {
> - /* If multiplying would overflow... */
> - if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1))
> - /* Divide first. */
> - return (user / XT_HASHLIMIT_SCALE_v1) *\
> - HZ * CREDITS_PER_JIFFY_v1;
> + if (revision == 1) {
> + /* If multiplying would overflow... */
> + if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1))
> + /* Divide first. */
> + return (user / XT_HASHLIMIT_SCALE_v1) *\
> + HZ * CREDITS_PER_JIFFY_v1;
> +
> + return (user * HZ * CREDITS_PER_JIFFY_v1) \
> + / XT_HASHLIMIT_SCALE_v1;
> + } else {
> + if (user > 0xFFFFFFFFFFFFFFFF / (HZ*CREDITS_PER_JIFFY))
> + return (user / XT_HASHLIMIT_SCALE) *\
> + HZ * CREDITS_PER_JIFFY;
>
> - return (user * HZ * CREDITS_PER_JIFFY_v1) / XT_HASHLIMIT_SCALE_v1;
> + return (user * HZ * CREDITS_PER_JIFFY) / XT_HASHLIMIT_SCALE;
> + }
> }
>
In my memory, 64-bit division operation should be replaced by
div_u64 or div64_u64, otherwise on some 32-bit architecture
systems, link error will happen. Something like this:
... undefined reference to `__udivdi3'.
^ permalink raw reply
* [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
From: fgao @ 2016-09-27 4:39 UTC (permalink / raw)
To: pablo, kaber, netfilter-devel, netdev; +Cc: gfree.wind, Gao Feng
From: Gao Feng <fgao@ikuai8.com>
Current xt_osf codes use memcmp to check if two user fingers are same,
so it depends on that the struct xt_osf_user_finger is no padding.
It is one implicit rule, and is not good to maintain.
Now use zero memory and assign the members explicitly.
Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
net/netfilter/xt_osf.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
index 2455b69..9793670 100644
--- a/net/netfilter/xt_osf.c
+++ b/net/netfilter/xt_osf.c
@@ -61,6 +61,34 @@ static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX + 1] = {
[OSF_ATTR_FINGER] = { .len = sizeof(struct xt_osf_user_finger) },
};
+static void copy_user_finger(struct xt_osf_user_finger *dst,
+ const struct xt_osf_user_finger *src)
+{
+#define OSF_COPY_MEMBER(mem) dst->mem = src->mem
+
+ int i;
+
+ OSF_COPY_MEMBER(wss.wc);
+ OSF_COPY_MEMBER(wss.val);
+
+ OSF_COPY_MEMBER(ttl);
+ OSF_COPY_MEMBER(df);
+ OSF_COPY_MEMBER(ss);
+ OSF_COPY_MEMBER(mss);
+ OSF_COPY_MEMBER(opt_num);
+
+ memcpy(dst->genre, src->genre, sizeof(dst->genre));
+ memcpy(dst->version, src->version, sizeof(dst->version));
+ memcpy(dst->subtype, src->subtype, sizeof(dst->subtype));
+
+ for (i = 0; i < MAX_IPOPTLEN; ++i) {
+ OSF_COPY_MEMBER(opt[i].kind);
+ OSF_COPY_MEMBER(opt[i].length);
+ OSF_COPY_MEMBER(opt[i].wc.wc);
+ OSF_COPY_MEMBER(opt[i].wc.val);
+ }
+}
+
static int xt_osf_add_callback(struct net *net, struct sock *ctnl,
struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const osf_attrs[])
@@ -77,11 +105,11 @@ static int xt_osf_add_callback(struct net *net, struct sock *ctnl,
f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
- kf = kmalloc(sizeof(struct xt_osf_finger), GFP_KERNEL);
+ kf = kzalloc(sizeof(*kf), GFP_KERNEL);
if (!kf)
return -ENOMEM;
- memcpy(&kf->finger, f, sizeof(struct xt_osf_user_finger));
+ copy_user_finger(&kf->finger, f);
list_for_each_entry(sf, &xt_osf_fingers[!!f->df], finger_entry) {
if (memcmp(&sf->finger, f, sizeof(struct xt_osf_user_finger)))
--
1.9.1
^ permalink raw reply related
* Re: ISDN-Gigaset: Fine-tuning for three function implementations
From: SF Markus Elfring @ 2016-09-27 5:10 UTC (permalink / raw)
To: Paul Bolle
Cc: gigaset307x-common, netdev, Karsten Keil, LKML, kernel-janitors,
Julia Lawall
In-Reply-To: <1474922316.8546.1.camel@tiscali.nl>
>> Use kmalloc_array() in two functions
>> Improve another size determination in gigaset_initcs()
>> Delete an error message for a failed memory allocation
>> Release memory in gigaset_initcs() after an allocation failure
>
> Which "static source code analysis" was used for that discovery?
Are you eventually asking more for the development tools which were involved here?
* Coccinelle software
* Script "checkpatch.pl"
* My own eyes with help of a current text editor and its programming support
Regards,
Markus
^ permalink raw reply
* Re: ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
From: SF Markus Elfring @ 2016-09-27 5:20 UTC (permalink / raw)
To: Paul Bolle
Cc: gigaset307x-common, netdev, Karsten Keil, LKML, kernel-janitors,
Julia Lawall
In-Reply-To: <1474924434.8546.29.camel@tiscali.nl>
>> Memory was not released (as it would be expected) when one call
>> of further resource reservations failed.
>
> This was the only thing in this series that triggered more than a,
> very uninspired, "meh" on first read.
Will it matter here if the function "kfree" will be called for the
data structure members "bcs" and "inbuf" after a later function call
failed within the implementation of "gigaset_initcs"?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v2 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: SF Markus Elfring @ 2016-09-27 5:32 UTC (permalink / raw)
To: David Miller
Cc: Joe Perches, David Laight, gigaset307x-common, netdev,
Karsten Keil, Paul Bolle, linux-kernel, kernel-janitors,
Julia Lawall
In-Reply-To: <20160926.201037.1525211304203135037.davem@davemloft.net>
> When you need to make changes to patches that are part of a series,
> you must resubmit the entire series,
I imagine that will happen when the patch review time passed by a bit
more as Paul Bolle requested it yesterday.
> not just the things that are changes.
Thanks for your reminder.
Regards,
Markus
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
From: Liping Zhang @ 2016-09-27 5:49 UTC (permalink / raw)
To: fgao; +Cc: Pablo Neira Ayuso, Patrick McHardy, netfilter-devel, netdev,
Feng Gao
In-Reply-To: <1474951193-3486-1-git-send-email-fgao@ikuai8.com>
Hi Feng,
2016-09-27 12:39 GMT+08:00 <fgao@ikuai8.com>:
> From: Gao Feng <fgao@ikuai8.com>
>
> Current xt_osf codes use memcmp to check if two user fingers are same,
> so it depends on that the struct xt_osf_user_finger is no padding.
> It is one implicit rule, and is not good to maintain.
>
> Now use zero memory and assign the members explicitly.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
> ---
> net/netfilter/xt_osf.c | 32 ++++++++++++++++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
> index 2455b69..9793670 100644
> --- a/net/netfilter/xt_osf.c
> +++ b/net/netfilter/xt_osf.c
> @@ -61,6 +61,34 @@ static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX + 1] = {
> [OSF_ATTR_FINGER] = { .len = sizeof(struct xt_osf_user_finger) },
> };
>
> +static void copy_user_finger(struct xt_osf_user_finger *dst,
> + const struct xt_osf_user_finger *src)
> +{
> +#define OSF_COPY_MEMBER(mem) dst->mem = src->mem
> +
> + int i;
> +
> + OSF_COPY_MEMBER(wss.wc);
> + OSF_COPY_MEMBER(wss.val);
> +
> + OSF_COPY_MEMBER(ttl);
> + OSF_COPY_MEMBER(df);
> + OSF_COPY_MEMBER(ss);
> + OSF_COPY_MEMBER(mss);
> + OSF_COPY_MEMBER(opt_num);
> +
> + memcpy(dst->genre, src->genre, sizeof(dst->genre));
> + memcpy(dst->version, src->version, sizeof(dst->version));
> + memcpy(dst->subtype, src->subtype, sizeof(dst->subtype));
> +
> + for (i = 0; i < MAX_IPOPTLEN; ++i) {
> + OSF_COPY_MEMBER(opt[i].kind);
> + OSF_COPY_MEMBER(opt[i].length);
> + OSF_COPY_MEMBER(opt[i].wc.wc);
> + OSF_COPY_MEMBER(opt[i].wc.val);
> + }
> +}
> +
This xt_osf_user_finger{} is carefully designed, no padding now, and
will not be changed in the future, otherwise backward compatibility will
be broken.
I don't think this convert is necessary, actually it is a little ugly, and will
increase the maintenance burden.
^ permalink raw reply
* Re: [PATCH net-next 4/4] net/sched: act_mirred: Implement ingress actions
From: David Miller @ 2016-09-27 5:56 UTC (permalink / raw)
To: shmulik.ladkani; +Cc: jhs, xiyou.wangcong, edumazet, netdev, shmulik.ladkani
In-Reply-To: <1474550512-7552-5-git-send-email-shmulik.ladkani@gmail.com>
From: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Date: Thu, 22 Sep 2016 16:21:52 +0300
> From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
>
> Up until now, 'action mirred' supported only egress actions (either
> TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).
>
> This patch implements the corresponding ingress actions
> TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
>
> This allows attaching filters whose target is to hand matching skbs into
> the rx processing of a specified device.
>
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
> Was wondering, whether netif_receive_skb or dev_forward_skb should be
> used for the rx bouncing. Used netif_receive_skb as in ifb device.
The discussion on this patch has ventured off into what to do about
recursion.
But it unclear to me where this specific patch, and this series,
stands right now. Someone please clear this up for me.
Thanks.
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
From: Gao Feng @ 2016-09-27 6:00 UTC (permalink / raw)
To: Liping Zhang
Cc: Pablo Neira Ayuso, Patrick McHardy,
Netfilter Developer Mailing List, Linux Kernel Network Developers
In-Reply-To: <CAML_gOcQt3MuDw3JAXYab1bKFaRuKgVO4jBaM22NOCShVoKmeA@mail.gmail.com>
Hi Liping,
On Tue, Sep 27, 2016 at 1:49 PM, Liping Zhang <zlpnobody@gmail.com> wrote:
> Hi Feng,
>
> 2016-09-27 12:39 GMT+08:00 <fgao@ikuai8.com>:
>> From: Gao Feng <fgao@ikuai8.com>
>>
>> Current xt_osf codes use memcmp to check if two user fingers are same,
>> so it depends on that the struct xt_osf_user_finger is no padding.
>> It is one implicit rule, and is not good to maintain.
>>
>> Now use zero memory and assign the members explicitly.
>>
>> Signed-off-by: Gao Feng <fgao@ikuai8.com>
>> ---
>> net/netfilter/xt_osf.c | 32 ++++++++++++++++++++++++++++++--
>> 1 file changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
>> index 2455b69..9793670 100644
>> --- a/net/netfilter/xt_osf.c
>> +++ b/net/netfilter/xt_osf.c
>> @@ -61,6 +61,34 @@ static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX + 1] = {
>> [OSF_ATTR_FINGER] = { .len = sizeof(struct xt_osf_user_finger) },
>> };
>>
>> +static void copy_user_finger(struct xt_osf_user_finger *dst,
>> + const struct xt_osf_user_finger *src)
>> +{
>> +#define OSF_COPY_MEMBER(mem) dst->mem = src->mem
>> +
>> + int i;
>> +
>> + OSF_COPY_MEMBER(wss.wc);
>> + OSF_COPY_MEMBER(wss.val);
>> +
>> + OSF_COPY_MEMBER(ttl);
>> + OSF_COPY_MEMBER(df);
>> + OSF_COPY_MEMBER(ss);
>> + OSF_COPY_MEMBER(mss);
>> + OSF_COPY_MEMBER(opt_num);
>> +
>> + memcpy(dst->genre, src->genre, sizeof(dst->genre));
>> + memcpy(dst->version, src->version, sizeof(dst->version));
>> + memcpy(dst->subtype, src->subtype, sizeof(dst->subtype));
>> +
>> + for (i = 0; i < MAX_IPOPTLEN; ++i) {
>> + OSF_COPY_MEMBER(opt[i].kind);
>> + OSF_COPY_MEMBER(opt[i].length);
>> + OSF_COPY_MEMBER(opt[i].wc.wc);
>> + OSF_COPY_MEMBER(opt[i].wc.val);
>> + }
>> +}
>> +
>
> This xt_osf_user_finger{} is carefully designed, no padding now, and
> will not be changed in the future, otherwise backward compatibility will
> be broken.
Yes, there is no padding now. So it is ok to use memcmp now.
I am afraid the struct would be modified for other requirements.
If it is never changed forever, it is ok certainly.
>
> I don't think this convert is necessary, actually it is a little ugly, and will
> increase the maintenance burden.
I just want the codes don't depend any implicit rule.
It is a tradeoff. If never change, needn't do any convert.
If may change, the memcmp is a little dangerous.
Regards
Feng
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
From: Liping Zhang @ 2016-09-27 6:05 UTC (permalink / raw)
To: Gao Feng
Cc: Pablo Neira Ayuso, Patrick McHardy,
Netfilter Developer Mailing List, Linux Kernel Network Developers
In-Reply-To: <CA+6hz4rZ5GF3MuVrywA87xQ-idq9iCL-cmmgA0sLwuHUp2PEbA@mail.gmail.com>
Hi Feng,
2016-09-27 14:00 GMT+08:00 Gao Feng <fgao@ikuai8.com>:
> Hi Liping,
>
>>
>> This xt_osf_user_finger{} is carefully designed, no padding now, and
>> will not be changed in the future, otherwise backward compatibility will
>> be broken.
>
> Yes, there is no padding now. So it is ok to use memcmp now.
> I am afraid the struct would be modified for other requirements.
This is structure was passed by netlink attribute, so modify it will
break backward compatibility.
>
> If it is never changed forever, it is ok certainly.
>
>>
>> I don't think this convert is necessary, actually it is a little ugly, and will
>> increase the maintenance burden.
>
> I just want the codes don't depend any implicit rule.
>
> It is a tradeoff. If never change, needn't do any convert.
> If may change, the memcmp is a little dangerous.
>
> Regards
> Feng
^ permalink raw reply
* Re: [PATCH nf-next] netfilter: xt_osf: Use explicit member assignment to avoid implicit no padding rule
From: Gao Feng @ 2016-09-27 6:10 UTC (permalink / raw)
To: Liping Zhang
Cc: Pablo Neira Ayuso, Patrick McHardy,
Netfilter Developer Mailing List, Linux Kernel Network Developers
In-Reply-To: <CAML_gOcVWerP2keCiqeTUt5QN-gbqATpTUsK-xPX=RnJoc8OBQ@mail.gmail.com>
Hi Liping,
On Tue, Sep 27, 2016 at 2:05 PM, Liping Zhang <zlpnobody@gmail.com> wrote:
> Hi Feng,
>
> 2016-09-27 14:00 GMT+08:00 Gao Feng <fgao@ikuai8.com>:
>> Hi Liping,
>>
>>>
>>> This xt_osf_user_finger{} is carefully designed, no padding now, and
>>> will not be changed in the future, otherwise backward compatibility will
>>> be broken.
>>
>> Yes, there is no padding now. So it is ok to use memcmp now.
>> I am afraid the struct would be modified for other requirements.
>
> This is structure was passed by netlink attribute, so modify it will
> break backward compatibility.
Reasonable.
Thanks Liping.
Regards
Feng
>
>>
>> If it is never changed forever, it is ok certainly.
>>
>>>
>>> I don't think this convert is necessary, actually it is a little ugly, and will
>>> increase the maintenance burden.
>>
>> I just want the codes don't depend any implicit rule.
>>
>> It is a tradeoff. If never change, needn't do any convert.
>> If may change, the memcmp is a little dangerous.
>>
>> Regards
>> Feng
^ permalink raw reply
* [PATCH] VSOCK: Don't dec ack backlog twice for rejected connections
From: Jorgen Hansen @ 2016-09-27 6:59 UTC (permalink / raw)
To: netdev, linux-kernel, virtualization
Cc: gregkh, davem, pv-drivers, stefanha, Jorgen Hansen
If a pending socket is marked as rejected, we will decrease the
sk_ack_backlog twice. So don't decrement it for rejected sockets
in vsock_pending_work().
Testing of the rejected socket path was done through code
modifications.
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jorgen Hansen <jhansen@vmware.com>
Reviewed-by: Adit Ranadive <aditr@vmware.com>
Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
---
net/vmw_vsock/af_vsock.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 17dbbe6..8a398b3 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -465,6 +465,8 @@ void vsock_pending_work(struct work_struct *work)
if (vsock_is_pending(sk)) {
vsock_remove_pending(listener, sk);
+
+ listener->sk_ack_backlog--;
} else if (!vsk->rejected) {
/* We are not on the pending list and accept() did not reject
* us, so we must have been accepted by our user process. We
@@ -475,8 +477,6 @@ void vsock_pending_work(struct work_struct *work)
goto out;
}
- listener->sk_ack_backlog--;
-
/* We need to remove ourself from the global connected sockets list so
* incoming packets can't find this socket, and to reduce the reference
* count.
@@ -2010,5 +2010,5 @@ EXPORT_SYMBOL_GPL(vsock_core_get_transport);
MODULE_AUTHOR("VMware, Inc.");
MODULE_DESCRIPTION("VMware Virtual Socket Family");
-MODULE_VERSION("1.0.1.0-k");
+MODULE_VERSION("1.0.2.0-k");
MODULE_LICENSE("GPL v2");
--
1.7.0
^ permalink raw reply related
* Re: [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: Dan Carpenter @ 2016-09-27 7:08 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: SF Markus Elfring, gigaset307x-common, netdev, Karsten Keil,
Paul Bolle, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <248e685c-4875-7acc-afaf-8b2ce00bb5f5@cogentembedded.com>
On Mon, Sep 26, 2016 at 08:38:14PM +0300, Sergei Shtylyov wrote:
> >@@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
> > {
> > unsigned char outbuf[80];
> > unsigned char c;
> >- size_t space = sizeof outbuf - 1;
> >+ size_t space = sizeof(outbuf - 1);
>
> What?! Does that compile?
>
> [...]
It prints a Smatch warning. Smatch ignores these if they happen inside
a macro where you pass a pointer and it takes the sizeof() the argument.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 4/5] ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
From: Dan Carpenter @ 2016-09-27 7:12 UTC (permalink / raw)
To: SF Markus Elfring
Cc: gigaset307x-common, netdev, Karsten Keil, Paul Bolle, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <6409b3b0-3b7a-1139-f040-7b1a61611187@users.sourceforge.net>
This patch creates new bugs.
I have a policy of not telling Markus where the bug is, because
otherwise he'll just resend the patch and I have told him many times to
stop sending these cleanup patches that just introduce bugs and waste
maintainer time.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses
From: Dan Carpenter @ 2016-09-27 7:25 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: SF Markus Elfring, gigaset307x-common, netdev, Karsten Keil,
Paul Bolle, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <20160927070837.GS13620@mwanda>
On Tue, Sep 27, 2016 at 10:08:37AM +0300, Dan Carpenter wrote:
> On Mon, Sep 26, 2016 at 08:38:14PM +0300, Sergei Shtylyov wrote:
> > >@@ -53,7 +53,7 @@ void gigaset_dbg_buffer(enum debuglevel level, const unsigned char *msg,
> > > {
> > > unsigned char outbuf[80];
> > > unsigned char c;
> > >- size_t space = sizeof outbuf - 1;
> > >+ size_t space = sizeof(outbuf - 1);
> >
> > What?! Does that compile?
> >
> > [...]
>
> It prints a Smatch warning. Smatch ignores these if they happen inside
> a macro where you pass a pointer and it takes the sizeof() the argument.
Reading that again, I realize it's not clear. Smatch ignores these any
time they happen in a macro whether they're valid or not. (Many times
they are valid).
regards,
dan carpenter
^ permalink raw reply
* Re: ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
From: SF Markus Elfring @ 2016-09-27 7:28 UTC (permalink / raw)
To: Dan Carpenter
Cc: gigaset307x-common, netdev, Karsten Keil, Paul Bolle, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <20160927071246.GT13620@mwanda>
> This patch creates new bugs.
Thanks for your information.
> I have a policy of not telling Markus where the bug is,
I find this kind of response strange.
> because otherwise he'll just resend the patch
This can also happen when the other contributors request it.
> and I have told him many times to stop sending these cleanup patches
Software "cleanups" seem to stress the review process to some degree.
> that just introduce bugs and waste maintainer time.
I guess that the situation is mixed depending on the subsystem
or concrete software module, isn't it?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH 4/5] ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
From: Dan Carpenter @ 2016-09-27 7:30 UTC (permalink / raw)
To: Paul Bolle
Cc: SF Markus Elfring, gigaset307x-common, netdev, Karsten Keil, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <1474924434.8546.29.camel@tiscali.nl>
Ah well... Someone else discovered the double free bug first and gave
it away. Reassuring, I guess.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH net v2] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
From: David Miller @ 2016-09-27 7:31 UTC (permalink / raw)
To: parameswaran.r7
Cc: kleptog, jchapman, netdev, linux-kernel, nprachan, rshearma,
dfawcus, stephen, acme, lboccass
In-Reply-To: <alpine.DEB.2.11.1609221341500.6975@duvvury30.eng.vyatta.net>
From: "R. Parameswaran" <parameswaran.r7@gmail.com>
Date: Thu, 22 Sep 2016 13:52:43 -0700 (PDT)
> From ed585bdd6d3d2b3dec58d414f514cd764d89159d Mon Sep 17 00:00:00 2001
> From: "R. Parameswaran" <rparames@brocade.com>
> Date: Thu, 22 Sep 2016 13:19:25 -0700
> Subject: [PATCH] L2TP:Adjust intf MTU,factor underlay L3,overlay L2
>
> Take into account all of the tunnel encapsulation headers when setting
> up the MTU on the L2TP logical interface device. Otherwise, packets
> created by the applications on top of the L2TP layer are larger
> than they ought to be, relative to the underlay MTU, leading to
> needless fragmentation once the outer IP encap is added.
>
> Specifically, take into account the (outer, underlay) IP header
> imposed on the encapsulated L2TP packet, and the Layer 2 header
> imposed on the inner IP packet prior to L2TP encapsulation.
>
> Do not assume an Ethernet (non-jumbo) underlay. Use the PMTU mechanism
> and the dst entry in the L2TP tunnel socket to directly pull up
> the underlay MTU (as the baseline number on top of which the
> encapsulation headers are factored in). Fall back to Ethernet MTU
> if this fails.
>
> Signed-off-by: R. Parameswaran <rparames@brocade.com>
>
> Reviewed-by: "N. Prachanda" <nprachan@brocade.com>,
> Reviewed-by: "R. Shearman" <rshearma@brocade.com>,
> Reviewed-by: "D. Fawcus" <dfawcus@brocade.com>
I have to ask, how do other tunnels over UDP such as VXLAN handle
this problem?
^ permalink raw reply
* Re: [PATCH 1/2] net: qcom/emac: do not use devm on internal phy pdev
From: David Miller @ 2016-09-27 7:34 UTC (permalink / raw)
To: timur; +Cc: netdev, linaro-acpi, linux-arm-kernel, shankerd, vikrams
In-Reply-To: <1474640854-1698-1-git-send-email-timur@codeaurora.org>
This patch doesn't apply to net-next.
Also, when you send a patch series, you must send an initial
posting with Subject of the form "[PATCH {net,net-next} 0/2] ..."
explaining at a high level what your patch series is doing,
how it is doing it, and why it is doing it that way.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 2/2] netfilter: Create revision 2 of xt_hashlimit to support higher pps rates
From: Vishwanath Pai @ 2016-09-27 7:35 UTC (permalink / raw)
To: Liping Zhang, Pablo Neira Ayuso
Cc: Vishwanath Pai, Patrick McHardy, Jozsef Kadlecsik, Josh Hunt,
netfilter-devel, coreteam, netdev
In-Reply-To: <CAML_gOdcrnmuHkxA1xAb2HDXHpo+=+sw+oJBfKXF3ijApsjTqg@mail.gmail.com>
On Tue, Sep 27, 2016 at 12:15 AM, Liping Zhang <zlpnobody@gmail.com> wrote:
> Hi Vishwanath,
>
> 2016-09-23 0:43 GMT+08:00 Vishwanath Pai <vpai@akamai.com>:
>>
>> /* Precision saver. */
>> -static u32 user2credits(u32 user)
>> +static u64 user2credits(u64 user, int revision)
>> {
>> - /* If multiplying would overflow... */
>> - if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1))
>> - /* Divide first. */
>> - return (user / XT_HASHLIMIT_SCALE_v1) *\
>> - HZ * CREDITS_PER_JIFFY_v1;
>> + if (revision == 1) {
>> + /* If multiplying would overflow... */
>> + if (user > 0xFFFFFFFF / (HZ*CREDITS_PER_JIFFY_v1))
>> + /* Divide first. */
>> + return (user / XT_HASHLIMIT_SCALE_v1) *\
>> + HZ * CREDITS_PER_JIFFY_v1;
>> +
>> + return (user * HZ * CREDITS_PER_JIFFY_v1) \
>> + / XT_HASHLIMIT_SCALE_v1;
>> + } else {
>> + if (user > 0xFFFFFFFFFFFFFFFF / (HZ*CREDITS_PER_JIFFY))
>> + return (user / XT_HASHLIMIT_SCALE) *\
>> + HZ * CREDITS_PER_JIFFY;
>>
>> - return (user * HZ * CREDITS_PER_JIFFY_v1) / XT_HASHLIMIT_SCALE_v1;
>> + return (user * HZ * CREDITS_PER_JIFFY) / XT_HASHLIMIT_SCALE;
>> + }
>> }
>>
>
> In my memory, 64-bit division operation should be replaced by
> div_u64 or div64_u64, otherwise on some 32-bit architecture
> systems, link error will happen. Something like this:
> ... undefined reference to `__udivdi3'.
I did not know that, thanks for pointing it out. I will send a patch
to fix this.
-Vishwanath
^ 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