diff mbox series

[net-next,v2,3/6] net: dcb: add new rewrite table

Message ID 20230116144853.2446315-4-daniel.machon@microchip.com (mailing list archive)
State New, archived
Headers show
Series Introduce new DCB rewrite table | expand

Commit Message

Daniel Machon Jan. 16, 2023, 2:48 p.m. UTC
Add new rewrite table and all the required functions, offload hooks and
bookkeeping for maintaining it. The rewrite table reuses the app struct,
and the entire set of app selectors. As such, some bookeeping code can
be shared between the rewrite- and the APP table.

New functions for getting, setting and deleting entries has been added.
Apart from operating on the rewrite list, these functions do not emit a
DCB_APP_EVENT when the list os modified. The new dcb_getrewr does a
lookup based on selector and priority and returns the protocol, so that
mappings from priority to protocol, for a given selector and ifindex is
obtained.

Also, a new nested attribute has been added, that encapsulates one or
more app structs. This attribute is used to distinguish the two tables.

The dcb_lock used for the APP table is reused for the rewrite table.

Signed-off-by: Daniel Machon <daniel.machon@microchip.com>
---
 include/net/dcbnl.h        |   8 +++
 include/uapi/linux/dcbnl.h |   2 +
 net/dcb/dcbnl.c            | 112 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 121 insertions(+), 1 deletion(-)

Comments

Petr Machata Jan. 18, 2023, 10:54 a.m. UTC | #1
Daniel Machon <daniel.machon@microchip.com> writes:

> diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
> index cb5319c6afe6..54af3ee03491 100644
> --- a/net/dcb/dcbnl.c
> +++ b/net/dcb/dcbnl.c
> @@ -178,6 +178,7 @@ static const struct nla_policy dcbnl_featcfg_nest[DCB_FEATCFG_ATTR_MAX + 1] = {
>  };
>  
>  static LIST_HEAD(dcb_app_list);
> +static LIST_HEAD(dcb_rewr_list);
>  static DEFINE_SPINLOCK(dcb_lock);
>  
>  static enum ieee_attrs_app dcbnl_app_attr_type_get(u8 selector)
> @@ -1138,7 +1139,7 @@ static int dcbnl_app_table_setdel(struct nlattr *attr,
>  static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
>  {
>  	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
> -	struct nlattr *ieee, *app;
> +	struct nlattr *ieee, *app, *rewr;
>  	struct dcb_app_type *itr;
>  	int dcbx;
>  	int err;
> @@ -1241,6 +1242,26 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
>  	spin_unlock_bh(&dcb_lock);
>  	nla_nest_end(skb, app);
>  
> +	rewr = nla_nest_start_noflag(skb, DCB_ATTR_DCB_REWR_TABLE);
> +	if (!rewr)
> +		return -EMSGSIZE;

This being new code, don't use _noflag please.

> +
> +	spin_lock_bh(&dcb_lock);
> +	list_for_each_entry(itr, &dcb_rewr_list, list) {
> +		if (itr->ifindex == netdev->ifindex) {
> +			enum ieee_attrs_app type =
> +				dcbnl_app_attr_type_get(itr->app.selector);
> +			err = nla_put(skb, type, sizeof(itr->app), &itr->app);
> +			if (err) {
> +				spin_unlock_bh(&dcb_lock);

This should cancel the nest started above.

I wonder if it would be cleaner in a separate function, so that there
can be a dedicated clean-up block to goto.

> +				return -EMSGSIZE;
> +			}
> +		}
> +	}
> +
> +	spin_unlock_bh(&dcb_lock);
> +	nla_nest_end(skb, rewr);
> +
>  	if (ops->dcbnl_getapptrust) {
>  		err = dcbnl_getapptrust(netdev, skb);
>  		if (err)
Daniel Machon Jan. 18, 2023, 1:47 p.m. UTC | #2
> > +     rewr = nla_nest_start_noflag(skb, DCB_ATTR_DCB_REWR_TABLE);
> > +     if (!rewr)
> > +             return -EMSGSIZE;
> 
> This being new code, don't use _noflag please.

Ack.

> 
> > +
> > +     spin_lock_bh(&dcb_lock);
> > +     list_for_each_entry(itr, &dcb_rewr_list, list) {
> > +             if (itr->ifindex == netdev->ifindex) {
> > +                     enum ieee_attrs_app type =
> > +                             dcbnl_app_attr_type_get(itr->app.selector);
> > +                     err = nla_put(skb, type, sizeof(itr->app), &itr->app);
> > +                     if (err) {
> > +                             spin_unlock_bh(&dcb_lock);
> 
> This should cancel the nest started above.

Yes, it should.

> 
> I wonder if it would be cleaner in a separate function, so that there
> can be a dedicated clean-up block to goto.

Well yes. That would make sense if the function were reused for both APP
and rewr.

Though in the APP equivalent code, nla_nest_start_noflag is used, and
dcbnl_ops->getdcbx() is called. Is there any userspace side-effect of
using nla_nest_start for APP too?

dcbnl_ops->getdcbx() would then be left outside of the shared function.
Does that call even have to hold the dcb_lock? Not as far as I can tell.

something like:

err = dcbnl_app_table_get(ndev, skb, &dcb_app_list,
			  DCB_ATTR_IEEE_APP_TABLE);
if (err)
	return -EMSGSIZE;

err = dcbnl_app_table_get(ndev, skb, &dcb_rewr_list,
			  DCB_ATTR_DCB_REWR_TABLE);
if (err)
        return -EMSGSIZE;

if (netdev->dcbnl_ops->getdcbx)
	dcbx = netdev->dcbnl_ops->getdcbx(netdev); <-- without lock held
else
	dcbx = -EOPNOTSUPP;

Let me hear your thoughts.
Dan Carpenter Jan. 18, 2023, 3:07 p.m. UTC | #3
On Wed, Jan 18, 2023 at 11:54:23AM +0100, Petr Machata wrote:
> > @@ -1241,6 +1242,26 @@ static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
> >  	spin_unlock_bh(&dcb_lock);
> >  	nla_nest_end(skb, app);
> >  
> > +	rewr = nla_nest_start_noflag(skb, DCB_ATTR_DCB_REWR_TABLE);
> > +	if (!rewr)
> > +		return -EMSGSIZE;
> 
> This being new code, don't use _noflag please.
> 
> > +
> > +	spin_lock_bh(&dcb_lock);
> > +	list_for_each_entry(itr, &dcb_rewr_list, list) {
> > +		if (itr->ifindex == netdev->ifindex) {
> > +			enum ieee_attrs_app type =
> > +				dcbnl_app_attr_type_get(itr->app.selector);
> > +			err = nla_put(skb, type, sizeof(itr->app), &itr->app);
> > +			if (err) {
> > +				spin_unlock_bh(&dcb_lock);
> 
> This should cancel the nest started above.
> 
> I wonder if it would be cleaner in a separate function, so that there
> can be a dedicated clean-up block to goto.
> 
> > +				return -EMSGSIZE;
> > +			}
> > +		}
> > +	}
> > +
> > +	spin_unlock_bh(&dcb_lock);
> > +	nla_nest_end(skb, rewr);

If you see a bug like this, you may as well ask Julia or me to add a
static checker warning for it.  We're both already on the CC list but we
might not be following the conversation closely...

In Smatch, I thought it would be easy but it turned out I need to add
a hack around to change the second nla_nest_start_noflag() from unknown
to valid pointer.

diff --git a/check_unwind.c b/check_unwind.c
index a397afd2346b..3128476cbeb6 100644
--- a/check_unwind.c
+++ b/check_unwind.c
@@ -94,6 +94,11 @@ static struct ref_func_info func_table[] = {
 
 	{ "ieee80211_alloc_hw", ALLOC,  -1, "$", &valid_ptr_min_sval, &valid_ptr_max_sval },
 	{ "ieee80211_free_hw",  RELEASE, 0, "$" },
+
+	{ "nla_nest_start_noflag", ALLOC, 0, "$", &valid_ptr_min_sval, &valid_ptr_max_sval },
+	{ "nla_nest_start", ALLOC, 0, "$", &valid_ptr_min_sval, &valid_ptr_max_sval },
+	{ "nla_nest_end", RELEASE, 0, "$" },
+	{ "nla_nest_cancel", RELEASE, 0, "$" },
 };
 
 static struct smatch_state *unmatched_state(struct sm_state *sm)
diff --git a/smatch_data/db/kernel.return_fixes b/smatch_data/db/kernel.return_fixes
index fa4ed4ba5f0f..4782c5e10cdb 100644
--- a/smatch_data/db/kernel.return_fixes
+++ b/smatch_data/db/kernel.return_fixes
@@ -90,3 +90,4 @@ dma_resv_wait_timeout s64min-(-1),1-s64max 1-s64max[<=$3]
 mmc_io_rw_direct_host s32min-(-1),1-s32max (-4095)-(-1)
 ad3552r_transfer s32min-(-1),1-s32max (-4095)-(-1)
 adin1110_read_reg s32min-(-1),1-s32max (-4095)-(-1)
+nla_nest_start_noflag 0-u64max 4096-ptr_max

Unfortunately, there is something weird going on and only my unreleased
version of Smatch finds the bug:

net/dcb/dcbnl.c:1306 dcbnl_ieee_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1160,1171,1184,1197,1207,1217,1222,1232,1257.
net/dcb/dcbnl.c:1502 dcbnl_cee_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1502.

I have been working on that check recently...  Both the released and
unreleased versions of Smatch have the following complaints:

net/dcb/dcbnl.c:400 dcbnl_getnumtcs() warn: 'skb' from nla_nest_start_noflag() not released on lines: 396.
net/dcb/dcbnl.c:1061 dcbnl_build_peer_app() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1061.
net/dcb/dcbnl.c:1359 dcbnl_cee_pg_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1324,1342.

regards,
dan carpenter
Dan Carpenter Jan. 18, 2023, 3:11 p.m. UTC | #4
On Wed, Jan 18, 2023 at 06:07:48PM +0300, Dan Carpenter wrote:
> In Smatch, I thought it would be easy but it turned out I need to add
> a hack around to change the second nla_nest_start_noflag() from unknown
> to valid pointer.

The second nla_nest_start_noflag() *return*, I meant.

> +nla_nest_start_noflag 0-u64max 4096-ptr_max

regards,
dan carpenter
Petr Machata Jan. 18, 2023, 3:20 p.m. UTC | #5
<Daniel.Machon@microchip.com> writes:

>> > +     rewr = nla_nest_start_noflag(skb, DCB_ATTR_DCB_REWR_TABLE);
>> > +     if (!rewr)
>> > +             return -EMSGSIZE;
>> 
>> This being new code, don't use _noflag please.
>
> Ack.
>
>> 
>> > +
>> > +     spin_lock_bh(&dcb_lock);
>> > +     list_for_each_entry(itr, &dcb_rewr_list, list) {
>> > +             if (itr->ifindex == netdev->ifindex) {
>> > +                     enum ieee_attrs_app type =
>> > +                             dcbnl_app_attr_type_get(itr->app.selector);
>> > +                     err = nla_put(skb, type, sizeof(itr->app), &itr->app);
>> > +                     if (err) {
>> > +                             spin_unlock_bh(&dcb_lock);
>> 
>> This should cancel the nest started above.
>
> Yes, it should.
>
>> 
>> I wonder if it would be cleaner in a separate function, so that there
>> can be a dedicated clean-up block to goto.
>
> Well yes. That would make sense if the function were reused for both APP
> and rewr.

I meant purely for to make the cleanup clear. The function would be
approximately:

static int dcbnl_ieee_fill_rewr(struct sk_buff *skb, struct net_device *netdev)
{
	struct dcb_app_type *itr;
	struct nlattr *rewr;
	int err;

	rewr = nla_nest_start_noflag(skb, DCB_ATTR_DCB_REWR_TABLE);
	if (!rewr)
		return -EMSGSIZE;

	spin_lock_bh(&dcb_lock);
	list_for_each_entry(itr, &dcb_rewr_list, list) {
		if (itr->ifindex == netdev->ifindex) {
			enum ieee_attrs_app type =
				dcbnl_app_attr_type_get(itr->app.selector);
			err = nla_put(skb, type, sizeof(itr->app), &itr->app);
			if (err)
				goto err_out;
		}
	}

	spin_unlock_bh(&dcb_lock);
	nla_nest_end(skb, rewr);
        return 0;

err_out:
	spin_unlock_bh(&dcb_lock);
	nla_nest_cancel(skb, rewr);
	return err;
}

Which uses an idiomatic style with the cleanup block at the end, instead
of stashing the individual cleanups before the return statement. I find
it easier to reason about.

But it's not a big deal. Your thing is readable just fine.

> Though in the APP equivalent code, nla_nest_start_noflag is used, and
> dcbnl_ops->getdcbx() is called. Is there any userspace side-effect of
> using nla_nest_start for APP too?

Yeah, the clients would be looking for code DCB_ATTR_IEEE_APP_TABLE, but
would get DCB_ATTR_IEEE_APP_TABLE | NLA_F_NESTED, and get confused.

For reuse between APP_TABLE and REWR_TABLE, you could just always call
_noflag in the helper, and pass the actual attribute in an argument.
Then the argument would be either DCB_ATTR_DCB_REWR_TABLE | NLA_F_NESTED,
or just plain DCB_ATTR_IEEE_APP_TABLE.

But that makes the code less clear, and I don't feel it brings much.

> dcbnl_ops->getdcbx() would then be left outside of the shared function.
> Does that call even have to hold the dcb_lock? Not as far as I can tell.
>
> something like:
>
> err = dcbnl_app_table_get(ndev, skb, &dcb_app_list,
> 			  DCB_ATTR_IEEE_APP_TABLE);
> if (err)
> 	return -EMSGSIZE;
>
> err = dcbnl_app_table_get(ndev, skb, &dcb_rewr_list,
> 			  DCB_ATTR_DCB_REWR_TABLE);
> if (err)
>         return -EMSGSIZE;
>
> if (netdev->dcbnl_ops->getdcbx)
> 	dcbx = netdev->dcbnl_ops->getdcbx(netdev); <-- without lock held
> else
> 	dcbx = -EOPNOTSUPP;
>
> Let me hear your thoughts.

Yeah, and the dcbx stuff is the added wrinkle.

Dunno, I'd not force it. This redundancy is not great, but the code is
small and easy to understand, so I find it's not an issue.
Petr Machata Jan. 19, 2023, 9:38 a.m. UTC | #6
Dan Carpenter <error27@gmail.com> writes:

> On Wed, Jan 18, 2023 at 11:54:23AM +0100, Petr Machata wrote:
>> > +
>> > +	spin_lock_bh(&dcb_lock);
>> > +	list_for_each_entry(itr, &dcb_rewr_list, list) {
>> > +		if (itr->ifindex == netdev->ifindex) {
>> > +			enum ieee_attrs_app type =
>> > +				dcbnl_app_attr_type_get(itr->app.selector);
>> > +			err = nla_put(skb, type, sizeof(itr->app), &itr->app);
>> > +			if (err) {
>> > +				spin_unlock_bh(&dcb_lock);
>> 
>> This should cancel the nest started above.
>> 
>
> If you see a bug like this, you may as well ask Julia or me to add a
> static checker warning for it.  We're both already on the CC list but we
> might not be following the conversation closely...

I'll try to remember next time.

> In Smatch, I thought it would be easy but it turned out I need to add
> a hack around to change the second nla_nest_start_noflag() from unknown
> to valid pointer.
>
> diff --git a/check_unwind.c b/check_unwind.c
> index a397afd2346b..3128476cbeb6 100644
> --- a/check_unwind.c
> +++ b/check_unwind.c
> @@ -94,6 +94,11 @@ static struct ref_func_info func_table[] = {
>  
>  	{ "ieee80211_alloc_hw", ALLOC,  -1, "$", &valid_ptr_min_sval, &valid_ptr_max_sval },
>  	{ "ieee80211_free_hw",  RELEASE, 0, "$" },
> +
> +	{ "nla_nest_start_noflag", ALLOC, 0, "$", &valid_ptr_min_sval, &valid_ptr_max_sval },
> +	{ "nla_nest_start", ALLOC, 0, "$", &valid_ptr_min_sval, &valid_ptr_max_sval },
> +	{ "nla_nest_end", RELEASE, 0, "$" },
> +	{ "nla_nest_cancel", RELEASE, 0, "$" },
>  };
>  
>  static struct smatch_state *unmatched_state(struct sm_state *sm)
> diff --git a/smatch_data/db/kernel.return_fixes b/smatch_data/db/kernel.return_fixes
> index fa4ed4ba5f0f..4782c5e10cdb 100644
> --- a/smatch_data/db/kernel.return_fixes
> +++ b/smatch_data/db/kernel.return_fixes
> @@ -90,3 +90,4 @@ dma_resv_wait_timeout s64min-(-1),1-s64max 1-s64max[<=$3]
>  mmc_io_rw_direct_host s32min-(-1),1-s32max (-4095)-(-1)
>  ad3552r_transfer s32min-(-1),1-s32max (-4095)-(-1)
>  adin1110_read_reg s32min-(-1),1-s32max (-4095)-(-1)
> +nla_nest_start_noflag 0-u64max 4096-ptr_max
>
> Unfortunately, there is something weird going on and only my unreleased
> version of Smatch finds the bug:
>
> net/dcb/dcbnl.c:1306 dcbnl_ieee_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1160,1171,1184,1197,1207,1217,1222,1232,1257.
> net/dcb/dcbnl.c:1502 dcbnl_cee_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1502.

Looking at a couple of those, yeah, it looks legit. Those are missing
the cancel on error returns.

> I have been working on that check recently...  Both the released and
> unreleased versions of Smatch have the following complaints:
>
> net/dcb/dcbnl.c:400 dcbnl_getnumtcs() warn: 'skb' from nla_nest_start_noflag() not released on lines: 396.
> net/dcb/dcbnl.c:1061 dcbnl_build_peer_app() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1061.
> net/dcb/dcbnl.c:1359 dcbnl_cee_pg_fill() warn: 'skb' from nla_nest_start_noflag() not released on lines: 1324,1342.

Likewise. Strange that each version reports a different subset. Or is
that just selective quoting?
diff mbox series

Patch

diff --git a/include/net/dcbnl.h b/include/net/dcbnl.h
index 8841ab6c2de7..fe7dfb8bcb5b 100644
--- a/include/net/dcbnl.h
+++ b/include/net/dcbnl.h
@@ -19,6 +19,10 @@  struct dcb_app_type {
 	u8	dcbx;
 };
 
+u16 dcb_getrewr(struct net_device *dev, struct dcb_app *app);
+int dcb_setrewr(struct net_device *dev, struct dcb_app *app);
+int dcb_delrewr(struct net_device *dev, struct dcb_app *app);
+
 int dcb_setapp(struct net_device *, struct dcb_app *);
 u8 dcb_getapp(struct net_device *, struct dcb_app *);
 int dcb_ieee_setapp(struct net_device *, struct dcb_app *);
@@ -113,6 +117,10 @@  struct dcbnl_rtnl_ops {
 	/* apptrust */
 	int (*dcbnl_setapptrust)(struct net_device *, u8 *, int);
 	int (*dcbnl_getapptrust)(struct net_device *, u8 *, int *);
+
+	/* rewrite */
+	int (*dcbnl_setrewr)(struct net_device *dev, struct dcb_app *app);
+	int (*dcbnl_delrewr)(struct net_device *dev, struct dcb_app *app);
 };
 
 #endif /* __NET_DCBNL_H__ */
diff --git a/include/uapi/linux/dcbnl.h b/include/uapi/linux/dcbnl.h
index 99047223ab26..7e15214aa5dd 100644
--- a/include/uapi/linux/dcbnl.h
+++ b/include/uapi/linux/dcbnl.h
@@ -411,6 +411,7 @@  enum dcbnl_attrs {
  * @DCB_ATTR_IEEE_PEER_PFC: peer PFC configuration - get only
  * @DCB_ATTR_IEEE_PEER_APP: peer APP tlv - get only
  * @DCB_ATTR_DCB_APP_TRUST_TABLE: selector trust table
+ * @DCB_ATTR_DCB_REWR_TABLE: rewrite configuration
  */
 enum ieee_attrs {
 	DCB_ATTR_IEEE_UNSPEC,
@@ -425,6 +426,7 @@  enum ieee_attrs {
 	DCB_ATTR_IEEE_QCN_STATS,
 	DCB_ATTR_DCB_BUFFER,
 	DCB_ATTR_DCB_APP_TRUST_TABLE,
+	DCB_ATTR_DCB_REWR_TABLE,
 	__DCB_ATTR_IEEE_MAX
 };
 #define DCB_ATTR_IEEE_MAX (__DCB_ATTR_IEEE_MAX - 1)
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index cb5319c6afe6..54af3ee03491 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -178,6 +178,7 @@  static const struct nla_policy dcbnl_featcfg_nest[DCB_FEATCFG_ATTR_MAX + 1] = {
 };
 
 static LIST_HEAD(dcb_app_list);
+static LIST_HEAD(dcb_rewr_list);
 static DEFINE_SPINLOCK(dcb_lock);
 
 static enum ieee_attrs_app dcbnl_app_attr_type_get(u8 selector)
@@ -1138,7 +1139,7 @@  static int dcbnl_app_table_setdel(struct nlattr *attr,
 static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
 {
 	const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops;
-	struct nlattr *ieee, *app;
+	struct nlattr *ieee, *app, *rewr;
 	struct dcb_app_type *itr;
 	int dcbx;
 	int err;
@@ -1241,6 +1242,26 @@  static int dcbnl_ieee_fill(struct sk_buff *skb, struct net_device *netdev)
 	spin_unlock_bh(&dcb_lock);
 	nla_nest_end(skb, app);
 
+	rewr = nla_nest_start_noflag(skb, DCB_ATTR_DCB_REWR_TABLE);
+	if (!rewr)
+		return -EMSGSIZE;
+
+	spin_lock_bh(&dcb_lock);
+	list_for_each_entry(itr, &dcb_rewr_list, list) {
+		if (itr->ifindex == netdev->ifindex) {
+			enum ieee_attrs_app type =
+				dcbnl_app_attr_type_get(itr->app.selector);
+			err = nla_put(skb, type, sizeof(itr->app), &itr->app);
+			if (err) {
+				spin_unlock_bh(&dcb_lock);
+				return -EMSGSIZE;
+			}
+		}
+	}
+
+	spin_unlock_bh(&dcb_lock);
+	nla_nest_end(skb, rewr);
+
 	if (ops->dcbnl_getapptrust) {
 		err = dcbnl_getapptrust(netdev, skb);
 		if (err)
@@ -1602,6 +1623,14 @@  static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
 			goto err;
 	}
 
+	if (ieee[DCB_ATTR_DCB_REWR_TABLE]) {
+		err = dcbnl_app_table_setdel(ieee[DCB_ATTR_DCB_REWR_TABLE],
+					     netdev,
+					     ops->dcbnl_setrewr ?: dcb_setrewr);
+		if (err)
+			goto err;
+	}
+
 	if (ieee[DCB_ATTR_IEEE_APP_TABLE]) {
 		err = dcbnl_app_table_setdel(ieee[DCB_ATTR_IEEE_APP_TABLE],
 					     netdev, ops->ieee_setapp ?:
@@ -1701,6 +1730,14 @@  static int dcbnl_ieee_del(struct net_device *netdev, struct nlmsghdr *nlh,
 			goto err;
 	}
 
+	if (ieee[DCB_ATTR_DCB_REWR_TABLE]) {
+		err = dcbnl_app_table_setdel(ieee[DCB_ATTR_DCB_REWR_TABLE],
+					     netdev,
+					     ops->dcbnl_delrewr ?: dcb_delrewr);
+		if (err)
+			goto err;
+	}
+
 err:
 	err = nla_put_u8(skb, DCB_ATTR_IEEE, err);
 	dcbnl_ieee_notify(netdev, RTM_SETDCB, DCB_CMD_IEEE_DEL, seq, 0);
@@ -1929,6 +1966,22 @@  static int dcb_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return ret;
 }
 
+static struct dcb_app_type *dcb_rewr_lookup(const struct dcb_app *app,
+					    int ifindex, int proto)
+{
+	struct dcb_app_type *itr;
+
+	list_for_each_entry(itr, &dcb_rewr_list, list) {
+		if (itr->app.selector == app->selector &&
+		    itr->app.priority == app->priority &&
+		    itr->ifindex == ifindex &&
+		    ((proto == -1) || itr->app.protocol == proto))
+			return itr;
+	}
+
+	return NULL;
+}
+
 static struct dcb_app_type *dcb_app_lookup(const struct dcb_app *app,
 					   int ifindex, int prio)
 {
@@ -2052,6 +2105,63 @@  u8 dcb_ieee_getapp_mask(struct net_device *dev, struct dcb_app *app)
 }
 EXPORT_SYMBOL(dcb_ieee_getapp_mask);
 
+/* Get protocol value from rewrite entry. */
+u16 dcb_getrewr(struct net_device *dev, struct dcb_app *app)
+{
+	struct dcb_app_type *itr;
+	u16 proto = 0;
+
+	spin_lock_bh(&dcb_lock);
+	itr = dcb_rewr_lookup(app, dev->ifindex, -1);
+	if (itr)
+		proto = itr->app.protocol;
+	spin_unlock_bh(&dcb_lock);
+
+	return proto;
+}
+EXPORT_SYMBOL(dcb_getrewr);
+
+ /* Add rewrite entry to the rewrite list. */
+int dcb_setrewr(struct net_device *dev, struct dcb_app *new)
+{
+	int err;
+
+	spin_lock_bh(&dcb_lock);
+	/* Search for existing match and abort if found. */
+	if (dcb_rewr_lookup(new, dev->ifindex, new->protocol)) {
+		err = -EEXIST;
+		goto out;
+	}
+
+	err = dcb_app_add(&dcb_rewr_list, new, dev->ifindex);
+out:
+	spin_unlock_bh(&dcb_lock);
+
+	return err;
+}
+EXPORT_SYMBOL(dcb_setrewr);
+
+/* Delete rewrite entry from the rewrite list. */
+int dcb_delrewr(struct net_device *dev, struct dcb_app *del)
+{
+	struct dcb_app_type *itr;
+	int err = -ENOENT;
+
+	spin_lock_bh(&dcb_lock);
+	/* Search for existing match and remove it. */
+	itr = dcb_rewr_lookup(del, dev->ifindex, del->protocol);
+	if (itr) {
+		list_del(&itr->list);
+		kfree(itr);
+		err = 0;
+	}
+
+	spin_unlock_bh(&dcb_lock);
+
+	return err;
+}
+EXPORT_SYMBOL(dcb_delrewr);
+
 /**
  * dcb_ieee_setapp - add IEEE dcb application data to app list
  * @dev: network interface